Weird layout of struct hal_rx_desc_qcn9274

[Karthikeyan Periyasamy]

On 9/5/2024 6:36 PM, Nicolas Escande wrote:
> 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 ?
> 

Yes

>>
>>
>>> 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 ?
> 

yes

>>>
>>> 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.

Not sure about the subscription in wcn7850
-- 
Karthikeyan Periyasamy
--
கார்த்திகேயன் பெரியசாமி