| Message ID | 20220812161003.27279-39-kvalo@kernel.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | wifi: ath12k: driver for Qualcomm Wi-Fi 7 devices | expand |
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. > + > +struct ath12k_peer { > + struct list_head list; > + struct ieee80211_sta *sta; > + int vdev_id; > + u8 addr[ETH_ALEN]; > + int peer_id; > + u16 ast_hash; > + u8 pdev_idx; > + u16 hw_peer_id; > + > + /* protected by ab->data_lock */ > + struct ieee80211_key_conf *keys[WMI_MAX_KEY_INDEX + 1]; > + struct ath12k_dp_rx_tid rx_tid[IEEE80211_NUM_TIDS + 1]; > + > + /* Info used in MMIC verification of > + * RX fragments > + */ > + struct crypto_shash *tfm_mmic; > + u8 mcast_keyidx; > + u8 ucast_keyidx; > + u16 sec_type; > + u16 sec_type_grp; > + struct ppdu_user_delayba ppdu_stats_delayba; > + bool delayba_flag; > + bool is_authorized; > +}; > + > +void ath12k_peer_unmap_event(struct ath12k_base *ab, u16 peer_id); > +void ath12k_peer_map_event(struct ath12k_base *ab, u8 vdev_id, u16 peer_id, > + u8 *mac_addr, u16 ast_hash, u16 hw_peer_id); > +struct ath12k_peer *ath12k_peer_find(struct ath12k_base *ab, int vdev_id, > + const u8 *addr); > +struct ath12k_peer *ath12k_peer_find_by_addr(struct ath12k_base *ab, > + const u8 *addr); > +struct ath12k_peer *ath12k_peer_find_by_id(struct ath12k_base *ab, int peer_id); > +void ath12k_peer_cleanup(struct ath12k *ar, u32 vdev_id); > +int ath12k_peer_delete(struct ath12k *ar, u32 vdev_id, u8 *addr); > +int ath12k_peer_create(struct ath12k *ar, struct ath12k_vif *arvif, > + struct ieee80211_sta *sta, > + struct ath12k_wmi_peer_create_arg *arg); > +int ath12k_wait_for_peer_delete_done(struct ath12k *ar, u32 vdev_id, > + const u8 *addr); > +struct ath12k_peer *ath12k_peer_find_by_vdev_id(struct ath12k_base *ab, > + int vdev_id); > +struct ath12k_peer *ath12k_peer_find_by_ast(struct ath12k_base *ab, int ast_hash); > + > +#endif /* _PEER_H_ */ >
>-----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
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; + +struct ath12k_peer { + struct list_head list; + struct ieee80211_sta *sta; + int vdev_id; + u8 addr[ETH_ALEN]; + int peer_id; + u16 ast_hash; + u8 pdev_idx; + u16 hw_peer_id; + + /* protected by ab->data_lock */ + struct ieee80211_key_conf *keys[WMI_MAX_KEY_INDEX + 1]; + struct ath12k_dp_rx_tid rx_tid[IEEE80211_NUM_TIDS + 1]; + + /* Info used in MMIC verification of + * RX fragments + */ + struct crypto_shash *tfm_mmic; + u8 mcast_keyidx; + u8 ucast_keyidx; + u16 sec_type; + u16 sec_type_grp; + struct ppdu_user_delayba ppdu_stats_delayba; + bool delayba_flag; + bool is_authorized; +}; + +void ath12k_peer_unmap_event(struct ath12k_base *ab, u16 peer_id); +void ath12k_peer_map_event(struct ath12k_base *ab, u8 vdev_id, u16 peer_id, + u8 *mac_addr, u16 ast_hash, u16 hw_peer_id); +struct ath12k_peer *ath12k_peer_find(struct ath12k_base *ab, int vdev_id, + const u8 *addr); +struct ath12k_peer *ath12k_peer_find_by_addr(struct ath12k_base *ab, + const u8 *addr); +struct ath12k_peer *ath12k_peer_find_by_id(struct ath12k_base *ab, int peer_id); +void ath12k_peer_cleanup(struct ath12k *ar, u32 vdev_id); +int ath12k_peer_delete(struct ath12k *ar, u32 vdev_id, u8 *addr); +int ath12k_peer_create(struct ath12k *ar, struct ath12k_vif *arvif, + struct ieee80211_sta *sta, + struct ath12k_wmi_peer_create_arg *arg); +int ath12k_wait_for_peer_delete_done(struct ath12k *ar, u32 vdev_id, + const u8 *addr); +struct ath12k_peer *ath12k_peer_find_by_vdev_id(struct ath12k_base *ab, + int vdev_id); +struct ath12k_peer *ath12k_peer_find_by_ast(struct ath12k_base *ab, int ast_hash); + +#endif /* _PEER_H_ */