Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <2a069aea-7b5d-ed12-c5ec-9c1de3b6a9ac@quicinc.com>
Date:   Wed, 9 Aug 2023 20:27:03 -0700
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v3] wifi: mwifiex: added code to support host mlme.
Content-Language: en-US
To:     David Lin <yu-hao.lin@nxp.com>,
        "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>
CC:     Sharvari Harisangam <sharvari.harisangam@nxp.com>,
        Pete Hsieh <tsung-hsien.hsieh@nxp.com>
References: <DU0PR04MB963607ADBFC49C7AB713092DD113A@DU0PR04MB9636.eurprd04.prod.outlook.com>
From:   Jeff Johnson <quic_jjohnson@quicinc.com>
In-Reply-To: <DU0PR04MB963607ADBFC49C7AB713092DD113A@DU0PR04MB9636.eurprd04.prod.outlook.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 8/9/2023 7:39 PM, David Lin wrote:
snip

> @@ -491,6 +511,17 @@ int mwifiex_cmd_802_11_associate(struct mwifiex_private *priv,
>                          sizeof(struct mwifiex_chan_scan_param_set);
>          }
> 
> +       if (priv->adapter->host_mlme) {
> +               host_mlme_tlv = (struct mwifiex_ie_types_host_mlme *)pos;
> +               host_mlme_tlv->header.type = cpu_to_le16(TLV_TYPE_HOST_MLME);
> +               host_mlme_tlv->header.len = sizeof(host_mlme_tlv->host_mlme);

cpu_to_le16(sizeof(host_mlme_tlv->host_mlme)) ??

> +               host_mlme_tlv->host_mlme = 1;
> +               pos += sizeof(host_mlme_tlv->header) +
> +                       host_mlme_tlv->header.len;

le16_to_cpu(host_mlme_tlv->header.len) ?? or just use 
sizeof(host_mlme_tlv->host_mlme)

> +               host_mlme_tlv->header.len =
> +                       cpu_to_le16(host_mlme_tlv->header.len);

The approach currently being taken doesn't look sparse-compliant. You 
should write an le16 into a __le16 member; writing a cpu16 into a __le16 
member and then converting in place using cpu_to_le16() is broken.

> +       }
> +

The above jumped out at me while scanning your patch -- it failed my 
mental pattern matching.

/jeff