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;
Message-ID: <8b456221-07aa-2620-784f-ae44ed2aa431@quicinc.com>
Date:   Mon, 15 Aug 2022 13:11:21 -0700
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.12.0
Subject: Re: [PATCH 10/50] wifi: ath12k: add debug.h
Content-Language: en-US
To:     Kalle Valo <kvalo@kernel.org>, <linux-wireless@vger.kernel.org>
CC:     <ath12k@lists.infradead.org>
References: <20220812161003.27279-1-kvalo@kernel.org>
 <20220812161003.27279-11-kvalo@kernel.org>
From:   Jeff Johnson <quic_jjohnson@quicinc.com>
In-Reply-To: <20220812161003.27279-11-kvalo@kernel.org>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

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/debug.h | 68 +++++++++++++++++++++++++++++++++
>   1 file changed, 68 insertions(+)
> 
> diff --git a/drivers/net/wireless/ath/ath12k/debug.h b/drivers/net/wireless/ath/ath12k/debug.h
> new file mode 100644
> index 000000000000..ec2be467edbc
> --- /dev/null
> +++ b/drivers/net/wireless/ath/ath12k/debug.h
> @@ -0,0 +1,68 @@
> +/* 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_DEBUG_H_
> +#define _ATH12K_DEBUG_H_
> +
> +#include "trace.h"
> +
> +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

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

> +	ATH12K_DBG_DP_RX	= 0x00002000,
> +	ATH12K_DBG_ANY		= 0xffffffff,
> +};
> +
> +__printf(2, 3) void ath12k_info(struct ath12k_base *ab, const char *fmt, ...);
> +__printf(2, 3) void ath12k_err(struct ath12k_base *ab, const char *fmt, ...);
> +__printf(2, 3) void ath12k_warn(struct ath12k_base *ab, const char *fmt, ...);
> +
> +extern unsigned int ath12k_debug_mask;
> +
> +#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

> +			       enum ath12k_debug_mask dbg_mask,
> +			       const char *fmt, ...)
> +{
> +	return 0;
> +}
> +
> +static inline 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)
> +{
> +}
> +#endif /* CONFIG_ATH12K_DEBUG */
> +
> +#define ath12k_dbg(ar, dbg_mask, fmt, ...)			\
> +do {								\
> +	typeof(dbg_mask) mask = (dbg_mask);			\
> +	if (ath12k_debug_mask & mask)				\
> +		__ath12k_dbg(ar, mask, fmt, ##__VA_ARGS__);	\
> +} while (0)
> +
> +#endif /* _ATH12K_DEBUG_H_ */
>