Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Kalle Valo <kvalo@kernel.org>
To:     Wen Gong <quic_wgong@quicinc.com>
Cc:     <ath11k@lists.infradead.org>, <linux-wireless@vger.kernel.org>
Subject: Re: [PATCH v4 2/5] wifi: ath11k: store cur_regulatory_info for each
 radio
References: <20230607094810.26707-1-quic_wgong@quicinc.com>
        <20230607094810.26707-3-quic_wgong@quicinc.com>
Date:   Wed, 02 Aug 2023 14:30:55 +0300
In-Reply-To: <20230607094810.26707-3-quic_wgong@quicinc.com> (Wen Gong's
        message of "Wed, 7 Jun 2023 05:48:07 -0400")
Message-ID: <878ratwuj4.fsf@kernel.org>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

Wen Gong <quic_wgong@quicinc.com> writes:

> The regulatory info of WMI_REG_CHAN_LIST_CC_EXT_EVENTID is not saved
> in ath11k now, the info should be saved in ath11k. Save the info for
> each radio and support switch regulatory rules dynamically.
>
> Tested-on: WCN6855 hw2.0 PCI
> WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3.6510.23
>
> Signed-off-by: Wen Gong <quic_wgong@quicinc.com>

[...]

> +enum wmi_vdev_type ath11k_mac_get_ar_vdev_type(struct ath11k *ar)
> +{
> +	struct ath11k_vif *arvif;
> +
> +	list_for_each_entry(arvif, &ar->arvifs, list) {
> +		return arvif->vdev_type;
> +	}
> +
> +	return WMI_VDEV_TYPE_UNSPEC;
> +}

This function looks odd to me and there are no comments to clarify.
What's the idea of using list_for_each_entry() and then immediately
return with the first entry? I guess the assumption here is that every
arvif has the same type? Can we really trust that? And at least it
should be documented here.

Also wouldn't list_first_entry_or_null() be more intuitive?

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches