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:     Jeff Johnson <quic_jjohnson@quicinc.com>
Cc:     <linux-wireless@vger.kernel.org>, <ath12k@lists.infradead.org>
Subject: Re: [PATCH 10/50] wifi: ath12k: add debug.h
References: <20220812161003.27279-1-kvalo@kernel.org>
        <20220812161003.27279-11-kvalo@kernel.org>
        <8b456221-07aa-2620-784f-ae44ed2aa431@quicinc.com>
Date:   Fri, 21 Oct 2022 14:18:49 +0300
In-Reply-To: <8b456221-07aa-2620-784f-ae44ed2aa431@quicinc.com> (Jeff
        Johnson's message of "Mon, 15 Aug 2022 13:11:21 -0700")
Message-ID: <87ilkdjug6.fsf@kernel.org>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

Jeff Johnson <quic_jjohnson@quicinc.com> writes:

> 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>

[...]

>> +enum ath12k_debug_mask {
>> +	ATH12K_DBG_AHB		= 0x00000001,
>> +	ATH12K_DBG_WMI		= 0x00000002,
>> +	ATH12K_DBG_HTC		= 0x00000004,
>> +	ATH12K_DBG_DP_HTT	= 0x00000008,
>> +	ATH12K_DBG_MAC		= 0x00000010,
>> +	ATH12K_DBG_BOOT		= 0x00000020,
>> +	ATH12K_DBG_QMI		= 0x00000040,
>> +	ATH12K_DBG_DATA		= 0x00000080,
>> +	ATH12K_DBG_MGMT		= 0x00000100,
>> +	ATH12K_DBG_REG		= 0x00000200,
>> +	ATH12K_DBG_TESTMODE	= 0x00000400,
>> +	ATH12K_DBG_HAL		= 0x00000800,
>> +	ATH12K_DBG_PCI		= 0x00001000,
>> +	ATH12K_DBG_DP_TX	= 0x00001000,
>
> duplicate assignments above

Praneesh fixed this:

c8bd8aa45f6a ath12k: fix ATH12K_DBG_DP_TX value in ath12k_debug_mask

> would using BIT() help here?
> or does having the hex masks make it easier for folks generating the
> masks

Back in the early ath10k days these enums were actually usign BIT() but
someone suggested to switch using hex numbers directly, just because
it's faster to calculate the debug mask. And I have to agree with that,
it's a lot easier when these are in hex.

>> +#ifdef CONFIG_ATH12K_DEBUG
>> +__printf(3, 4) void __ath12k_dbg(struct ath12k_base *ab,
>> +				 enum ath12k_debug_mask mask,
>> +				 const char *fmt, ...);
>> +void ath12k_dbg_dump(struct ath12k_base *ab,
>> +		     enum ath12k_debug_mask mask,
>> +		     const char *msg, const char *prefix,
>> +		     const void *buf, size_t len);
>> +#else /* CONFIG_ATH12K_DEBUG */
>> +static inline int __ath12k_dbg(struct ath12k_base *ab,
>
> should be a void function with an empty body to match the signature of
> the real function

Praneesh fixed in:

c568d8bc26de ath12k: convert payload[0] to payload[], use ab variable and fix __ath12k_dbg() signature

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

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