Re: [PATCH v5 2/9] wifi: ath12k: Add HAL_PHYRX_GENERIC_EHT_SIG TLV parsing support

[Karthikeyan Periyasamy <quic_periyasa@xxxxxxxxxxx>]

On 1/29/2025 10:58 AM, Vasanthakumar Thiagarajan wrote:
> On 1/27/2025 4:17 PM, Karthikeyan Periyasamy wrote:
> > Currently, monitor is not enabled. However, in the future, the monitor
> > will be enabled. Therefore, add the necessary HAL_PHYRX_GENERIC_EHT_SIG
> > TLV parsing support in monitor Rx path, which help to populate the EHT
> > radiotap data.
> >
> > Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.3.1-00173-QCAHKSWPL_SILICONZ-1
> > Tested-on: WCN7850 hw2.0 PCI 
> > WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
> >
> > Co-developed-by: P Praneesh <quic_ppranees@xxxxxxxxxxx>
> > Signed-off-by: P Praneesh <quic_ppranees@xxxxxxxxxxx>
> > Signed-off-by: Karthikeyan Periyasamy <quic_periyasa@xxxxxxxxxxx>
> > ---
> >   drivers/net/wireless/ath/ath12k/dp_mon.c | 571 +++++++++++++++++++++--
> >   drivers/net/wireless/ath/ath12k/hal_rx.h | 136 +++++-
> >   2 files changed, 661 insertions(+), 46 deletions(-)

...

> > +
> > +static enum hal_rx_mon_status
> > +ath12k_dp_mon_hal_rx_parse_eht_sig_non_ofdma(const void *tlv,
> > +                         struct hal_rx_mon_ppdu_info *ppdu_info)
> > +{
> > +    const struct hal_eht_sig_non_ofdma_cmn_eb *eb = tlv;
> > +
> > +    ath12k_dp_mon_hal_rx_parse_usig_overflow(tlv, ppdu_info);
> > +    ath12k_dp_mon_hal_rx_parse_non_ofdma_users(eb, ppdu_info);
> > +
> > +    if (ath12k_dp_mon_hal_rx_is_mu_mimo_user(&ppdu_info->u_sig_info))
> > +        
> > ath12k_dp_mon_hal_rx_parse_eht_mumimo_user(&eb->user_field.mu_mimo,
> > +                               ppdu_info);
> > +    else
> > +        
> > ath12k_dp_mon_hal_rx_parse_eht_non_mumimo_user(&eb->user_field.n_mu_mimo,
> > +                                   ppdu_info);
> > +
> > +    return HAL_TLV_STATUS_PPDU_NOT_DONE;
> > +}
> > +
> > +static enum hal_rx_mon_status
> > +ath12k_dp_mon_hal_rx_parse_ru_allocation(const struct 
> > hal_eht_sig_ofdma_cmn_eb *eb,
> > +                     struct hal_rx_mon_ppdu_info *ppdu_info)
> > +{
> > +    const struct hal_eht_sig_ofdma_cmn_eb1 *ofdma_cmn_eb1 = &eb->eb1;
> > +    const struct hal_eht_sig_ofdma_cmn_eb2 *ofdma_cmn_eb2 = &eb->eb2;
> > +    struct hal_rx_radiotap_eht *eht = &ppdu_info->eht_info.eht;
> > +    enum ieee80211_radiotap_eht_data ru_123, ru_124, ru_125, ru_126;
> > +    enum ieee80211_radiotap_eht_data ru_121, ru_122, ru_112, ru_111;
> > +    u32 data;
> > +
> > +    ru_123 = IEEE80211_RADIOTAP_EHT_DATA4_RU_ALLOC_CC_1_2_3;
> > +    ru_124 = IEEE80211_RADIOTAP_EHT_DATA5_RU_ALLOC_CC_1_2_4;
> > +    ru_125 = IEEE80211_RADIOTAP_EHT_DATA5_RU_ALLOC_CC_1_2_5;
> > +    ru_126 = IEEE80211_RADIOTAP_EHT_DATA6_RU_ALLOC_CC_1_2_6;
> > +    ru_121 = IEEE80211_RADIOTAP_EHT_DATA3_RU_ALLOC_CC_1_2_1;
> > +    ru_122 = IEEE80211_RADIOTAP_EHT_DATA3_RU_ALLOC_CC_1_2_2;
> > +    ru_112 = IEEE80211_RADIOTAP_EHT_DATA2_RU_ALLOC_CC_1_1_2;
> > +    ru_111 = IEEE80211_RADIOTAP_EHT_DATA1_RU_ALLOC_CC_1_1_1;
> > +
> > +    switch (ppdu_info->u_sig_info.bw) {
> > +    case HAL_EHT_BW_320_2:
> > +    case HAL_EHT_BW_320_1:
> > +        data = __le32_to_cpu(eht->data[4]);
> > +        /* CC1 2::3 */
> > +        data |=    
> > IEEE80211_RADIOTAP_EHT_DATA4_RU_ALLOC_CC_1_2_3_KNOWN |
> > +            ATH12K_LE64_DEC_ENC(ofdma_cmn_eb2->info0,
> > +                        HAL_RX_EHT_SIG_OFDMA_EB2_RU_ALLOC_2_3,
> > +                        ru_123);
> > +        eht->data[4] = cpu_to_le32(data);
> > +
> > +        data = __le32_to_cpu(eht->data[5]);
> > +        /* CC1 2::4 */
> > +        data |=    
> > IEEE80211_RADIOTAP_EHT_DATA5_RU_ALLOC_CC_1_2_4_KNOWN |
> > +            ATH12K_LE64_DEC_ENC(ofdma_cmn_eb2->info0,
> > +                        HAL_RX_EHT_SIG_OFDMA_EB2_RU_ALLOC_2_4,
> > +                        ru_124);
> > +
> > +        /* CC1 2::5 */
> > +        data |=    
> > IEEE80211_RADIOTAP_EHT_DATA5_RU_ALLOC_CC_1_2_5_KNOWN |
> > +            ATH12K_LE64_DEC_ENC(ofdma_cmn_eb2->info0,
> > +                        HAL_RX_EHT_SIG_OFDMA_EB2_RU_ALLOC_2_5,
> > +                        ru_125);
> > +        eht->data[5] = cpu_to_le32(data);
> > +
> > +        data = __le32_to_cpu(eht->data[6]);
> > +        /* CC1 2::6 */
> > +        data |=    
> > IEEE80211_RADIOTAP_EHT_DATA6_RU_ALLOC_CC_1_2_6_KNOWN |
> > +            ATH12K_LE64_DEC_ENC(ofdma_cmn_eb2->info0,
> > +                        HAL_RX_EHT_SIG_OFDMA_EB2_RU_ALLOC_2_6,
> > +                        ru_126);
> > +        eht->data[6] = cpu_to_le32(data);
> > +
> > +        fallthrough;
> > +    case HAL_EHT_BW_160:
> > +        data = __le32_to_cpu(eht->data[3]);
> > +        /* CC1 2::1 */
> > +        data |=    
> > IEEE80211_RADIOTAP_EHT_DATA3_RU_ALLOC_CC_1_2_1_KNOWN |
> > +            ATH12K_LE64_DEC_ENC(ofdma_cmn_eb2->info0,
> > +                        HAL_RX_EHT_SIG_OFDMA_EB2_RU_ALLOC_2_1,
> > +                        ru_121);
> > +        /* CC1 2::2 */
> > +        data |=    
> > IEEE80211_RADIOTAP_EHT_DATA3_RU_ALLOC_CC_1_2_2_KNOWN |
> > +            ATH12K_LE64_DEC_ENC(ofdma_cmn_eb2->info0,
> > +                        HAL_RX_EHT_SIG_OFDMA_EB2_RU_ALLOC_2_2,
> > +                        ru_122);
> > +        eht->data[3] = cpu_to_le32(data);
> > +
> > +        fallthrough;
> > +    case HAL_EHT_BW_80:
> > +        data = __le32_to_cpu(eht->data[2]);
> > +        /* CC1 1::2 */
> > +        data |=    
> > IEEE80211_RADIOTAP_EHT_DATA2_RU_ALLOC_CC_1_1_2_KNOWN |
> > +            ATH12K_LE64_DEC_ENC(ofdma_cmn_eb1->info0,
> > +                        HAL_RX_EHT_SIG_OFDMA_EB1_RU_ALLOC_1_2,
> > +                        ru_112);
> > +        eht->data[2] = cpu_to_le32(data);
> > +
> > +        fallthrough;
> > +    case HAL_EHT_BW_40:
> > +        fallthrough;
> > +    case HAL_EHT_BW_20:
> > +        data = __le32_to_cpu(eht->data[1]);
> > +        /* CC1 1::1 */
> > +        data |=    
> > IEEE80211_RADIOTAP_EHT_DATA1_RU_ALLOC_CC_1_1_1_KNOWN |
> > +            ATH12K_LE64_DEC_ENC(ofdma_cmn_eb1->info0,
> > +                        HAL_RX_EHT_SIG_OFDMA_EB1_RU_ALLOC_1_1,
> > +                        ru_111);
> > +        eht->data[1] = cpu_to_le32(data);
> > +        break;
> > +    default:
> > +        break;
> > +    }
> > +
> > +    return HAL_TLV_STATUS_PPDU_NOT_DONE;
> > +}
> > +
> > +static enum hal_rx_mon_status
> > +ath12k_dp_mon_hal_rx_parse_eht_sig_ofdma(const void *tlv,
> > +                     struct hal_rx_mon_ppdu_info *ppdu_info)
> > +{
> > +    const struct hal_eht_sig_ofdma_cmn_eb *ofdma = tlv;
> > +
> > +    ath12k_dp_mon_hal_rx_parse_usig_overflow(tlv, ppdu_info);
> > +    ath12k_dp_mon_hal_rx_parse_ru_allocation(ofdma, ppdu_info);
> > +
> > +    
> > ath12k_dp_mon_hal_rx_parse_eht_non_mumimo_user(&ofdma->user_field.n_mu_mimo,
> > +                               ppdu_info);
> > +
> > +    return HAL_TLV_STATUS_PPDU_NOT_DONE;
> > +}
> > +
>
> Same comment as the previous one, make it void as the return code
> seems not very much useful.

sure, will fix in the next version of the patch.

--
Karthikeyan Periyasamy
--
கார்த்திகேயன் பெரியசாமி