Re: Weird layout of struct hal_rx_desc_qcn9274

["Nicolas Escande" <nico.escande@gmail.com>]

On 2024-09-05 4:53, Karthikeyan Periyasamy wrote:

[...]

> > 
> > 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 ?
> > 
> 
> yes, qcn9274 not packed with __le64 tag.
> wcn7850 expect the __le64 tag packed in the msdu_end and mpdu_start.

So in the full form, we just do not have the _le64 tag on qcn9274 but not in
wcn7850, right ? 

> 
> 
> > 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 ?
> 
> Yes, tag tlv is configurable in the compact word subscription mode.
> Before compact mode, qcn9274 full mode is worked without any issue.
>

So that means that setting all bits in the tlv subscription mode *is not* the
same as not using the subscription and receiving the full desciptor. Setting all
bits will get you the __le64 tag, but not using the registration at all will not
get it right ? 

> > 
> > 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 ?
> > 
> 
> Either way, __le64 msdu_end_tag is part of the msdu_end because 
> ath12k_hw_wcn7850_rx_desc_get_msdu_end_offset() tells the 
> offsetof(struct hal_rx_desc_wcn7850, msdu_end_tag) not the 
> offsetof(struct rx_msdu_end_wcn7850 msdu_end) in the 
> ath12k_dp_rxdma_ring_sel_config_wcn7850() configuration.
> 
Yep I saw this part, for me it accentuates the fact that the msdu_end_tag is not
something that can subscribed out of in wcn7850, as it is out of the struct,
but can in qcn9274.