Re: [PATCH] brcmfmac: support STA info struct v7

[Arend van Spriel <arend.vanspriel@broadcom.com>]

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