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:     Ping-Ke Shih <pkshih@realtek.com>
Cc:     <linux-wireless@vger.kernel.org>
Subject: Re: [PATCH] wifi: rtw89: 8852b: change debug mask of message of no TX resource
References: <20221103072517.15284-1-pkshih@realtek.com>
Date:   Mon, 07 Nov 2022 14:32:43 +0200
In-Reply-To: <20221103072517.15284-1-pkshih@realtek.com> (Ping-Ke Shih's
        message of "Thu, 3 Nov 2022 15:25:17 +0800")
Message-ID: <87sfivt05w.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

Ping-Ke Shih <pkshih@realtek.com> writes:

> 8852B has smaller TX FIFO than others in WiFi chip, so it would be buffer
> full frequently, but it doesn't affect TX performance. However, it shows
> verbose debug messages with RTW89_DBG_UNEXP mask that is to show up
> abnormal behavior, so change debug mask to RTW89_DBG_TXRX for 8852B.
>
> Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
> ---
>  drivers/net/wireless/realtek/rtw89/pci.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtw89/pci.c b/drivers/net/wireless/realtek/rtw89/pci.c
> index 07a2e23759f0b..99a254d7ba5ed 100644
> --- a/drivers/net/wireless/realtek/rtw89/pci.c
> +++ b/drivers/net/wireless/realtek/rtw89/pci.c
> @@ -971,8 +971,10 @@ static u32 __rtw89_pci_check_and_reclaim_tx_resource(struct rtw89_dev *rtwdev,
>  	struct rtw89_pci *rtwpci = (struct rtw89_pci *)rtwdev->priv;
>  	struct rtw89_pci_tx_ring *tx_ring = &rtwpci->tx_rings[txch];
>  	struct rtw89_pci_tx_wd_ring *wd_ring = &tx_ring->wd_ring;
> +	const struct rtw89_chip_info *chip = rtwdev->chip;
>  	u32 bd_cnt, wd_cnt, min_cnt = 0;
>  	struct rtw89_pci_rx_ring *rx_ring;
> +	enum rtw89_debug_mask debug_mask;
>  	u32 cnt;
>  
>  	rx_ring = &rtwpci->rx_rings[RTW89_RXCH_RPQ];
> @@ -996,10 +998,16 @@ static u32 __rtw89_pci_check_and_reclaim_tx_resource(struct rtw89_dev *rtwdev,
>  	bd_cnt = rtw89_pci_get_avail_txbd_num(tx_ring);
>  	wd_cnt = wd_ring->curr_num;
>  	min_cnt = min(bd_cnt, wd_cnt);
> -	if (min_cnt == 0)
> -		rtw89_debug(rtwdev, rtwpci->low_power ? RTW89_DBG_TXRX : RTW89_DBG_UNEXP,
> +	if (min_cnt == 0) {
> +		if (rtwpci->low_power || chip->chip_id == RTL8852B)
> +			debug_mask = RTW89_DBG_TXRX;
> +		else
> +			debug_mask = RTW89_DBG_UNEXP;
> +
> +		rtw89_debug(rtwdev, debug_mask,
>  			    "still no tx resource after reclaim: wd_cnt=%d bd_cnt=%d\n",
>  			    wd_cnt, bd_cnt);
> +	}

Changing the debug mask like this looks a bit weird to me. At least it
would be good to have a comment in the code explainin why this is done
like this.

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

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