RE: [PATCH 38/50] wifi: ath12k: add peer.h

Sriram R (QUIC)
2022-10-12T23:13:19Z

>-----Original Message-----
>From: Jeff Johnson <quic_jjohnson@quicinc.com>
>Sent: Thursday, August 18, 2022 12:55 AM
>To: Kalle Valo <kvalo@kernel.org>; linux-wireless@vger.kernel.org
>Cc: ath12k@lists.infradead.org
>Subject: Re: [PATCH 38/50] wifi: ath12k: add peer.h
>
>WARNING: This email originated from outside of Qualcomm. Please be wary
>of any links or attachments, and do not enable macros.
>
>On 8/12/2022 9:09 AM, Kalle Valo wrote:
>> From: Kalle Valo <quic_kvalo@quicinc.com>
>>
>> (Patches split into one patch per file for easier review, but the
>> final commit will be one big patch. See the cover letter for more
>> info.)
>>
>> Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>
>> ---
>> drivers/net/wireless/ath/ath12k/peer.h | 69
>++++++++++++++++++++++++++++++++++
>> 1 file changed, 69 insertions(+)
>>
>> diff --git a/drivers/net/wireless/ath/ath12k/peer.h
>> b/drivers/net/wireless/ath/ath12k/peer.h
>> new file mode 100644
>> index 000000000000..d0346e8b1b60
>> --- /dev/null
>> +++ b/drivers/net/wireless/ath/ath12k/peer.h
>> @@ -0,0 +1,69 @@
>> +/* SPDX-License-Identifier: BSD-3-Clause-Clear */
>> +/*
>> + * Copyright (c) 2018-2021 The Linux Foundation. All rights reserved.
>> + * Copyright (c) 2021-2022 Qualcomm Innovation Center, Inc. All rights
>reserved.
>> + */
>> +
>> +#ifndef ATH12K_PEER_H
>> +#define ATH12K_PEER_H
>> +
>> +#include "dp_rx.h"
>> +
>> +struct ppdu_user_delayba {
>> + u8 reserved0;
>> + u16 sw_peer_id;
>> + u32 info0;
>> + u16 ru_end;
>> + u16 ru_start;
>> + u32 info1;
>> + u32 rate_flags;
>> + u32 resp_rate_flags;
>> +} __packed;
>
>(reviewing this out of order since I saw strange usage in dp_rx.c review)
>
>the above struct seems to be a host-only struct that is only written by
>ath12k_copy_to_delay_stats() and only read by ath12k_copy_to_bar().
>Therefore use of __packed seems unnecessary, especially since due to the
>layout that will cause many members to be unaligned. In addition the
>reserved0 member is not used, and since this is a host-only struct, is
>unnecessary.
>
>an alternative implementation would be to use the actual hardware
>definition here, struct htt_ppdu_stats_user_rate, and then functions
>ath12k_copy_to_delay_stats() and ath12k_copy_to_bar() could be simple
>struct assignments.
>
Sure Jeff. Will address it.

Thanks,
Sriram.R