Weird layout of struct hal_rx_desc_qcn9274

[Nicolas Escande]

So I think I've stumbled upon another weird thing.

In rx_desc.h the definition of hal_rx_desc as used the firmware seems to be
varying depending on the HW and on the use of reduced metadata (with some tlv
registration magic that seems to select the info we retrieve, 8bytes at a time)  

// For QCN9274 full metadata:

struct rx_msdu_end_qcn9274 {
	__le16 info0;
	__le16 phy_ppdu_id;
	__le16 ip_hdr_cksum;
	__le16 info1;
...
};

struct hal_rx_desc_qcn9274 {
	struct rx_msdu_end_qcn9274 msdu_end;
	struct rx_mpdu_start_qcn9274 mpdu_start;
	u8 msdu_payload[];
} __packed;


// For QCN9274 with reduced metadata:

struct rx_msdu_end_qcn9274_compact {
	__le64 msdu_end_tag;
	__le16 sa_sw_peer_id;
	__le16 info5;
...
};

struct hal_rx_desc_qcn9274_compact {
	struct rx_msdu_end_qcn9274_compact msdu_end;
	struct rx_mpdu_start_qcn9274_compact mpdu_start;
	u8 msdu_payload[];
} __packed;


// For WCN7850 full metadata:

struct rx_msdu_end_wcn7850 {
	__le16 info0;
	__le16 phy_ppdu_id;
	__le16 ip_hdr_cksum;
	__le16 info1;
...
};

struct hal_rx_desc_wcn7850 {
	__le64 msdu_end_tag;
	struct rx_msdu_end_wcn7850 msdu_end;
	u8 rx_padding0[RX_BE_PADDING0_BYTES];
	__le64 mpdu_start_tag;
	struct rx_mpdu_start_qcn9274 mpdu_start;
	struct rx_pkt_hdr_tlv	 pkt_hdr_tlv;
	u8 msdu_payload[];
};


Which gets then used as a generic rx desc

struct hal_rx_desc {
	union {
		struct hal_rx_desc_qcn9274 qcn9274;
		struct hal_rx_desc_qcn9274_compact qcn9274_compact;
		struct hal_rx_desc_wcn7850 wcn7850;
	} u;
} __packed;



So it seems without the quadword registration, even though qcn9274 & wcn7850
appears to have similar layout, the __le64 msdu_end_tag is present in the struct
hal_rx_desc_wcn7850 but not in struct hal_rx_desc_qcn9274. That seems weird.
Would this mean that firmware always sends 8bytes of msdu_end_tag in wcn7850 but
not on qcn9274 ?

It seems unlikely as when using the compact layout on qcn9274 (the default now)
we use QCN9274_MSDU_END_WMASK with QCN9274_MSDU_END_SELECT_MSDU_END_TAG (bit(0))
which seems to indicate that this 8bytes tag is part of the optional data that
we can select (or not) to be received from the firmware.

Am I missing something ? Did that even worked before switching to compact layout
for qcn9274 ?

And if it's not working as intended, what's the layout style we actually want to
use for struct hal_rx_desc_XXX, do we want the __le64 msdu_end_tag inside the
struct hal_rx_desc_XXX or inside the struct rx_msdu_end_XXX ?

Thanks