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:     Pkshih <pkshih@realtek.com>
Cc:     "Larry.Finger\@lwfinger.net" <Larry.Finger@lwfinger.net>,
        "linux-wireless\@vger.kernel.org" <linux-wireless@vger.kernel.org>
Subject: Re: Funny log entries for rtw88_8821ce
References: <c356d5ae-a7b3-3065-1121-64c446e70333@lwfinger.net>
        <32f2ece8acdb67f74757cb705e5467847d6bcae0.camel@realtek.com>
        <ef7361eb-17ce-5ecf-a46a-a9f0c5aab35b@lwfinger.net>
        <87czjxbukz.fsf@tynnyri.adurom.net>
        <6baf4ae2f36c89269f74f0905ce81f38adc3a284.camel@realtek.com>
Date:   Thu, 10 Feb 2022 16:26:57 +0200
In-Reply-To: <6baf4ae2f36c89269f74f0905ce81f38adc3a284.camel@realtek.com>
        (Pkshih's message of "Tue, 8 Feb 2022 12:02:33 +0000")
Message-ID: <87mtiy3iha.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

Pkshih <pkshih@realtek.com> writes:

>> -----Original Message-----
>> From: Kalle Valo <kvalo@kernel.org>
>> Sent: Tuesday, February 8, 2022 5:02 PM
>> To: Larry Finger <Larry.Finger@lwfinger.net>
>> Cc: Pkshih <pkshih@realtek.com>; linux-wireless@vger.kernel.org
>> Subject: Re: Funny log entries for rtw88_8821ce
>> 
>> Larry Finger <Larry.Finger@lwfinger.net> writes:
>> 
>> > From these log entries, I see no reason why there should be a stop-vif
>> > 412 seconds after the interface is set from disabled to disconnected.
>> > This cycling does not cause any problems. Perhaps the statements that
>> > do the logging should be removed.
>> 
>> I did a quick check on how rtw88 uses rtw_info() and indeed that should
>> be cleaned up.
>> 
>> These should be warning or error messages:
>> 
>> rtw_info(rtwdev, "invalid H2C command format for debug\n");
>> rtw_info(rtwdev, "incorrect lna index (%d)\n", lna_idx);
>> rtw_info(rtwdev, "unsupported tx path 0x%x\n", antenna_tx);
>> rtw_info(rtwdev, "unsupported rx path 0x%x\n", antenna_rx);
>> rtw_info(rtwdev, "unsupported tx path 0x%x\n", antenna_tx);
>> rtw_info(rtwdev, "unsupported rx path 0x%x\n", antenna_rx);
>> 
>> Not sure if warn or debug, most likely debug:
>> 
>> rtw_info(rtwdev, "HW scan aborted with code: %d\n", rc);
>> 
>> These should be debug messages to avoid spamming the logs:
>> 
>> rtw_info(rtwdev, "start vif %pM on port %d\n", vif->addr, rtwvif->port);
>> rtw_info(rtwdev, "stop vif %pM on port %d\n", vif->addr, rtwvif->port);
>> rtw_info(rtwdev, "change vif %pM (%d)->(%d), p2p (%d)->(%d)\n",
>> rtw_info(rtwdev, "sta %pM joined with macid %d\n",
>> rtw_info(rtwdev, "sta %pM with macid %d left\n",
>> rtw_info(rtwdev, "On freq %u to %u, set SAR %d in 1/%lu dBm\n"
>> 
>> This is ok:
>> 
>> rtw_info(rtwdev, "Firmware version %u.%u.%u, H2C version %u\n",
>> 
>
> I will fix them as well as rtw89. To avoid mistakes, I do google search for
> KERN_INFO, and find [1]
> 	KERN_INFO: this is the log level used for informational messages
> 		   about the action performed by the kernel.

Not following you here.

> Then, I have a question about the message of setting SAR:
> 	rtw_info(rtwdev, "On freq %u to %u, set SAR %d in 1/%lu dBm\n"
>
> When a user sets SAR via iw, this message can reflect the action performed
> by driver. Is this rtw_info acceptable?

In general the preference for user space commands is not to print
anything to the log when debug messages are disabled, but of course
there can be exceptions if the reasons are good. Why do you want to
print this always? What benefit does it bring for the user?

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

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