Re: [PATCH v4 01/12] wifi: ath12k: add multiple radio support in a single MAC HW un/register

Jeff Johnson
2024-03-12T21:53:09Z

On 3/12/2024 6:55 AM, Rameshkumar Sundaram wrote:
> From: Karthikeyan Periyasamy <quic_periyasa@quicinc.com>
>
> Currently MAC HW un/register helper function support the single radio.
> To enable single/multi link operation in the future, the following helper
> functions need to be refactored to accommodate multiple radios under a
> single MAC HW un/register:
>
> * ath12k_ah_to_ar()
> * ath12k_mac_hw_allocate()
> * ath12k_mac_hw_register()
> * ath12k_mac_hw_unregister()
>
> This refactoring will make it easier to scale these functionalities and
> support Multi link operation.
>
> Current Multi wiphy Model
>
> +---------------+ +---------------+ +---------------+
> | Mac80211 hw | | Mac80211 hw | | Mac80211 hw |
> | private data | | private data | | private data |
> | | | | | |
> |ath12k_hw (ah) | |ath12k_hw (ah) | |ath12k_hw (ah) |
> | | | | | |
> | +-----------+ | | +-----------+ | | +-----------+ |
> | | ar (2GHz) | | | | ar (5GHz) | | | | ar (6GHz) | |
> | +-----------+ | | +-----------+ | | +-----------+ |
> | | | | | |
> +---------------+ +---------------+ +---------------+
>
> Single wiphy Model
>
> +--------------+
> | Mac80211 hw |
> | private data |
> | |
> |ath12k hw (ah)|
> | +----------+ |
> | |ar (2GHz) | |
> | +----------+ |
> | |ar (5GHz) | |
> | +----------+ |
> | |ar (6GHz) | |
> | +----------+ |
> +--------------+
>
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
>
> Signed-off-by: Karthikeyan Periyasamy <quic_periyasa@quicinc.com>
> Signed-off-by: Sriram R <quic_srirrama@quicinc.com>
> Signed-off-by: Rameshkumar Sundaram <quic_ramess@quicinc.com>
> ---
> drivers/net/wireless/ath/ath12k/core.h | 12 +-
> drivers/net/wireless/ath/ath12k/mac.c | 184 ++++++++++++++++---------
> drivers/net/wireless/ath/ath12k/reg.c | 2 +-
> 3 files changed, 127 insertions(+), 71 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h
> index 97e5a0ccd233..ff831faa4945 100644
> --- a/drivers/net/wireless/ath/ath12k/core.h
> +++ b/drivers/net/wireless/ath/ath12k/core.h
> @@ -951,13 +951,21 @@ static inline struct ath12k_hw *ath12k_hw_to_ah(struct ieee80211_hw *hw)
> return hw->priv;
> }
>
> -static inline struct ath12k *ath12k_ah_to_ar(struct ath12k_hw *ah)
> +static inline struct ath12k *ath12k_ah_to_ar(struct ath12k_hw *ah, u8 hw_link_id)
> {
> - return ah->radio;
> + if (WARN(hw_link_id >= ah->num_radio,
> + "bad hw link id %d, so switch to default link\n", hw_link_id))
> + hw_link_id = 0;
> +
> + return &ah->radio[hw_link_id];
> }
>
> static inline struct ieee80211_hw *ath12k_ar_to_hw(struct ath12k *ar)
> {
> return ar->ah->hw;
> }
> +
> +#define for_each_ar(index, ah, ar) \
> + for ((index) = 0; ((index) < (ah)->num_radio && \
> + ((ar) = &(ah)->radio[(index)])); (index)++)

this seems like logically the wrong order of operands
this is an operation on the ah object so IMO that should be first
the actual iterators i and ar should follow that

and guess we have to figure out how to suppress the ath12k-check issues with
this macro

that's my only comments on this patch.

/jeff