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 02/15] ath11k: Add support to parse new wmi event for 6 GHz regulatory
References: <20211026111913.7346-1-quic_wgong@quicinc.com>
        <20211026111913.7346-3-quic_wgong@quicinc.com>
Date:   Thu, 09 Dec 2021 19:21:20 +0200
In-Reply-To: <20211026111913.7346-3-quic_wgong@quicinc.com> (Wen Gong's
        message of "Tue, 26 Oct 2021 07:19:00 -0400")
Message-ID: <87bl1pwv73.fsf@codeaurora.org>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

Wen Gong <quic_wgong@quicinc.com> writes:

> In order to support different power levels of 6 GHz AP and client, new
> wmi event for regulatory was added in firmware(WMI_REG_CHAN_LIST_CC_EXT_EVENTID)
> to provide new parameters required for 6 GHz regulatory rules.
>
> firmware advertises its capability of handling new event in wmi service ready
> event. Based on that, host needs to set host_service_flags in wmi init
> command to indicate that host supports processing of new wmi event.
> Based on advertised host capability, firmware decides to send old event
> (WMI_REG_CHAN_LIST_CC_EVENTID) or new event(WMI_REG_CHAN_LIST_CC_EXT_EVENTID).
>
> Add support for parsing 2.4 GHz/5 GHz/6 GHz reg rules and other parameters from
> WMI_REG_CHAN_LIST_CC_EXT_EVENTID, to populate the channel lists.
> Since 6 GHz requires additional power value fields(PSD info), update
> reg rule parsing function.
>
> Signed-off-by: Lavanya Suresh <quic_lavaks@quicinc.com>
> Signed-off-by: Wen Gong <quic_wgong@quicinc.com>

[...]

> --- a/drivers/net/wireless/ath/ath11k/core.c
> +++ b/drivers/net/wireless/ath/ath11k/core.c
> @@ -230,6 +230,7 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
>  		.supports_suspend = true,
>  		.hal_desc_sz = sizeof(struct hal_rx_desc_wcn6855),
>  		.fix_l1ss = false,
> +		.support_ext_cc = true,
>  	},
>  };

Please add all the false cases as well. I'm trying to keep all hardware
entries have all fields specified.

> diff --git a/drivers/net/wireless/ath/ath11k/core.h b/drivers/net/wireless/ath/ath11k/core.h
> index 67b843a92344..a65f7d00eea2 100644
> --- a/drivers/net/wireless/ath/ath11k/core.h
> +++ b/drivers/net/wireless/ath/ath11k/core.h
> @@ -966,4 +966,10 @@ static inline const char *ath11k_bus_str(enum ath11k_bus bus)
>  	return "unknown";
>  }
>  
> +static inline bool ath11k_support_cc_ext(struct ath11k_base *ab)
> +{
> +	return ab->hw_params.support_ext_cc &&
> +	       test_bit(WMI_TLV_SERVICE_REG_CC_EXT_EVENT_SUPPORT, ab->wmi_ab.svc_map);
> +}

Add core:

ath11k_core_supports_cc_ext()

Or maybe it would be more intuitive to have this in hw.h?

ath11k_hw_supports_cc_ext()

> diff --git a/drivers/net/wireless/ath/ath11k/hw.h b/drivers/net/wireless/ath/ath11k/hw.h
> index b17c37e9eda7..86fd9dc82983 100644
> --- a/drivers/net/wireless/ath/ath11k/hw.h
> +++ b/drivers/net/wireless/ath/ath11k/hw.h
> @@ -164,6 +164,7 @@ struct ath11k_hw_params {
>  	bool supports_suspend;
>  	u32 hal_desc_sz;
>  	bool fix_l1ss;
> +	bool support_ext_cc;

supports_ext_cc

But in ath11k_core_supports_cc_ext() you cc before ext, maybe it should
be the same here as well? Something like:

supports_cc_ext

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

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