DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 573E820827
Subject: Re: [PATCH] brcmfmac: support STA info struct v7
To:     Dan Haab <riproute@gmail.com>, Kalle Valo <kvalo@codeaurora.org>
References: <1539289141-13689-1-git-send-email-dan.haab@luxul.com>
Cc:     Franky Lin <franky.lin@broadcom.com>,
        Hante Meuleman <hante.meuleman@broadcom.com>,
        Chi-Hsien Lin <chi-hsien.lin@cypress.com>,
        Wright Feng <wright.feng@cypress.com>,
        "David S. Miller" <davem@davemloft.net>,
        =?UTF-8?B?UmFmYcWCIE1pxYJlY2tp?= <rafal@milecki.pl>,
        Chung-Hsien Hsu <stanley.hsu@cypress.com>,
        linux-wireless@vger.kernel.org,
        brcm80211-dev-list.pdl@broadcom.com,
        brcm80211-dev-list@cypress.com, Dan Haab <dan.haab@luxul.com>
From:   Arend van Spriel <arend.vanspriel@broadcom.com>
Message-ID: <d4de8ccb-0de9-bf55-53d9-79f3d5f7df4e@broadcom.com>
Date:   Wed, 7 Nov 2018 22:03:10 +0100
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101
 Thunderbird/45.8.0
MIME-Version: 1.0
In-Reply-To: <1539289141-13689-1-git-send-email-dan.haab@luxul.com>
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 7bit
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

On 10/11/2018 10:19 PM, Dan Haab wrote:
> The newest firmwares provide STA info using v7 of the struct. As v7
> isn't backward compatible, a union is needed.
>
> Even though brcmfmac does not use any of the new info it's important to
> provide the proper struct buffer. Without this change new firmwares will
> fallback to the very limited v3 instead of something in between such as
> v4.
>
> Signed-off-by: Dan Haab <dan.haab@luxul.com>
> ---
>  .../broadcom/brcm80211/brcmfmac/fwil_types.h       | 39 ++++++++++++++++++----
>  1 file changed, 32 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
> index d5bb81e..189d576 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
> @@ -176,6 +176,8 @@
>
>  #define BRCMF_VHT_CAP_MCS_MAP_NSS_MAX	8
>
> +#define BRCMF_HE_CAP_MCS_MAP_NSS_MAX	8
> +
>  /* MAX_CHUNK_LEN is the maximum length for data passing to firmware in each
>   * ioctl. It is relatively small because firmware has small maximum size input
>   * playload restriction for ioctls.
> @@ -601,13 +603,36 @@ struct brcmf_sta_info_le {
>  	__le32 rx_pkts_retried;        /* # rx with retry bit set */
>  	__le32 tx_rate_fallback;       /* lowest fallback TX rate */
>
> -	/* Fields valid for ver >= 5 */
> -	struct {
> -		__le32 count;					/* # rates in this set */
> -		u8 rates[BRCMF_MAXRATES_IN_SET];		/* rates in 500kbps units w/hi bit set if basic */
> -		u8 mcs[BRCMF_MCSSET_LEN];			/* supported mcs index bit map */
> -		__le16 vht_mcs[BRCMF_VHT_CAP_MCS_MAP_NSS_MAX];	/* supported mcs index bit map per nss */
> -	} rateset_adv;
> +	union {
> +		struct {
> +			struct {
> +				__le32 count;					/* # rates in this set */
> +				u8 rates[BRCMF_MAXRATES_IN_SET];		/* rates in 500kbps units w/hi bit set if basic */
> +				u8 mcs[BRCMF_MCSSET_LEN];			/* supported mcs index bit map */
> +				__le16 vht_mcs[BRCMF_VHT_CAP_MCS_MAP_NSS_MAX];	/* supported mcs index bit map per nss */
> +			} rateset_adv;
> +		} v5;
> +
> +		struct {
> +			__le32 rx_dur_total;	/* total user RX duration (estimated) */
> +			__le16 chanspec;	/** chanspec this sta is on */
> +			__le16 pad;

Here we add explicity padding field.

> +			struct {
> +				__le16 version;					/* version */
> +				__le16 len;					/* length */
> +				__le32 count;					/* # rates in this set */
> +				u8 rates[BRCMF_MAXRATES_IN_SET];		/* rates in 500kbps units w/hi bit set if basic */
> +				u8 mcs[BRCMF_MCSSET_LEN];			/* supported mcs index bit map */
> +				__le16 vht_mcs[BRCMF_VHT_CAP_MCS_MAP_NSS_MAX];	/* supported mcs index bit map per nss */
> +				__le16 he_mcs[BRCMF_HE_CAP_MCS_MAP_NSS_MAX];	/* supported he mcs index bit map per nss */
> +			} rateset_adv;		/* rateset along with mcs index bitmap */
> +			__le16 wpauth;		/* authentication type */
> +			u8 algo;		/* crypto algorithm */

So I prefer to have padding explicit here as well, ie. a u8 field.

> +			__le32 tx_rspec;	/* Rate of last successful tx frame */
> +			__le32 rx_rspec;	/* Rate of last successful rx frame */
> +			__le32 wnm_cap;		/* wnm capabilities */
> +		} v7;
> +	};
>  };

Regards,
Arend