Re: [PATCH 03/31] wifi: mwifiex: drop HostCmd_CMD_802_11_MAC_ADDRESS response handling

[Brian Norris <briannorris@chromium.org>]

Hi Sascha,

On Tue, Aug 20, 2024 at 01:55:28PM +0200, Sascha Hauer wrote:
> The command response handler copies the new MAC address over to
> priv->curr_addr. The same is done in the code issuing the call
> already, so drop the unnecessary HostCmd_CMD_802_11_MAC_ADDRESS
> handling.

It took a bit to figure out what you meant here -- I guess you're
referring to mwifiex_set_mac_address()? It could help to document what
you mean.

I'm also a bit torn; this command API ostensibly has a (unused so far,
for this command) HostCmd_ACT_GEN_GET mode, in which case this *is*
important.

If anything, I might consider dropping some of the handling in
mwifiex_set_mac_address(), because it seems to presume (and then has to
undo for failure) behavior of the underlying command.

Brian

> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c | 20 --------------------
>  1 file changed, 20 deletions(-)
> 
> diff --git a/drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c b/drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c
> index 9c53825f222d1..7f81e709bd6b7 100644
> --- a/drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c
> +++ b/drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c
> @@ -473,25 +473,6 @@ static int mwifiex_ret_rf_antenna(struct mwifiex_private *priv,
>  	return 0;
>  }
>  
> -/*
> - * This function handles the command response of set/get MAC address.
> - *
> - * Handling includes saving the MAC address in driver.
> - */
> -static int mwifiex_ret_802_11_mac_address(struct mwifiex_private *priv,
> -					  struct host_cmd_ds_command *resp)
> -{
> -	struct host_cmd_ds_802_11_mac_address *cmd_mac_addr =
> -							&resp->params.mac_addr;
> -
> -	memcpy(priv->curr_addr, cmd_mac_addr->mac_addr, ETH_ALEN);
> -
> -	mwifiex_dbg(priv->adapter, INFO,
> -		    "info: set mac address: %pM\n", priv->curr_addr);
> -
> -	return 0;
> -}
> -
>  /*
>   * This function handles the command response of set/get MAC multicast
>   * address.
> @@ -1232,7 +1213,6 @@ int mwifiex_process_sta_cmdresp(struct mwifiex_private *priv, u16 cmdresp_no,
>  	case HostCmd_CMD_MAC_CONTROL:
>  		break;
>  	case HostCmd_CMD_802_11_MAC_ADDRESS:
> -		ret = mwifiex_ret_802_11_mac_address(priv, resp);
>  		break;
>  	case HostCmd_CMD_MAC_MULTICAST_ADR:
>  		ret = mwifiex_ret_mac_multicast_adr(priv, resp);
> 
> -- 
> 2.39.2
>