On 19/03/2024 11:18, Ping-Ke Shih wrote:
> On Sun, 2024-03-17 at 20:44 +0200, Bitterblue Smith wrote:
>>
>> v2:
>> - Add cover letter.
>> - Implement feedback.
>> - Fix more problems reported by checkpatch.
>> - Split the new driver into several patches (4-12) for easier
>> reviewing.
>> - More details about the changes can be found in each patch.
>>
>>
>
> I have not started reviewing yet, but compiler reports errors/warnings:
>
> drivers/net/wireless/realtek/rtlwifi/rtl8192du/phy.c: In function 'rtl92d_phy_set_poweron':
> drivers/net/wireless/realtek/rtlwifi/rtl8192du/phy.c:3055:1: error: expected declaration or
> statement at end of input
> 3055 | }
> | ^
> At top level:
>
Ahh, that's embarrassing. checkpatch said "else" after "break"
is not useful, so I removed the else. I didn't notice the open
brace after the if... and forgot to compile again before
format-patch. Sorry about that.
> And, sparse/smatch report
>
I installed sparse and smatch now.
> drivers/net/wireless/realtek/rtlwifi/rtl8192d/phy_common.h:30:17: warning: 'channel_all' defined
> but not used [-Wunused-const-variable=]
> drivers/net/wireless/realtek/rtlwifi/rtl8192de/../rtl8192d/phy_common.h:30:17: warning:
> 'channel_all' defined but not used [-Wunused-const-variable=]
I see now that channel_all is only used in phy_common.c.
I will move it there.
> drivers/net/wireless/realtek/rtlwifi/rtl8192d/phy_common.h:60:39: warning: context imbalance in
> 'rtl92d_bandtype_2_4G' - unexpected unlock
> drivers/net/wireless/realtek/rtlwifi/rtl8192d/phy_common.h:60:39: warning: context imbalance in
> 'rtl92d_dm_false_alarm_counter_statistics' - unexpected unlock
> drivers/net/wireless/realtek/rtlwifi/rtl8192d/phy_common.h:60:39: warning: context imbalance in
> 'rtl92d_dm_cck_packet_detection_thresh' - unexpected unlock
These look like false positives. Every unlock is preceded by
a lock. I found a suggestion to annotate the functions with
"__acquires(...)" and "__releases(...)" to quiet these warnings,
but that didn't do anything. I can only fix it by copying the
contents of rtl92d_acquire_cckandrw_pagea_ctl() and
rtl92d_release_cckandrw_pagea_ctl() to the eight places where
they are called, and duplicating the code that needs locking:
if (rtlpriv->rtlhal.interfaceindex == 1 &&
rtlpriv->rtlhal.interface == INTF_PCI) {
spin_lock_irqsave(&rtlpriv->locks.cck_and_rw_pagea_lock, flag);
temp_cck = rtl_get_bbreg(hw, RCCK0_TXFILTER2,
MASKDWORD) & MASKCCK;
spin_unlock_irqrestore(&rtlpriv->locks.cck_and_rw_pagea_lock, flag);
} else {
temp_cck = rtl_get_bbreg(hw, RCCK0_TXFILTER2,
MASKDWORD) & MASKCCK;
}
> drivers/net/wireless/realtek/rtlwifi/rtl8192de/../rtl8192d/phy_common.h:30:17: warning:
> 'channel_all' defined but not used [-Wunused-const-variable=]
> drivers/net/wireless/realtek/rtlwifi/rtl8192du/../rtl8192d/phy_common.h:30:17: warning:
> 'channel_all' defined but not used [-Wunused-const-variable=]
> drivers/net/wireless/realtek/rtlwifi/rtl8192d/phy_common.h:30:17: warning: 'channel_all' defined
> but not used [-Wunused-const-variable=]
> drivers/net/wireless/realtek/rtlwifi/rtl8192de/../rtl8192d/phy_common.h:30:17: warning:
> 'channel_all' defined but not used [-Wunused-const-variable=]
> drivers/net/wireless/realtek/rtlwifi/rtl8192de/../rtl8192d/phy_common.h:60:39: warning: context
> imbalance in 'rtl92d_phy_set_bw_mode' - unexpected unlock
> drivers/net/wireless/realtek/rtlwifi/rtl8192de/../rtl8192d/phy_common.h:60:39: warning: context
> imbalance in '_rtl92d_phy_reload_imr_setting' - unexpected unlock
> drivers/net/wireless/realtek/rtlwifi/rtl8192de/../rtl8192d/phy_common.h:60:39: warning: context
> imbalance in 'rtl92d_phy_iq_calibrate' - unexpected unlock
> drivers/net/wireless/realtek/rtlwifi/rtl8192du/../rtl8192d/phy_common.h:30:17: warning:
> 'channel_all' defined but not used [-Wunused-const-variable=]
> drivers/net/wireless/realtek/rtlwifi/rtl8192de/../rtl8192d/phy_common.h:30:17: warning:
> 'channel_all' defined but not used [-Wunused-const-variable=]
> drivers/net/wireless/realtek/rtlwifi/rtl8192de/../rtl8192d/phy_common.h:30:17: warning:
> 'channel_all' defined but not used [-Wunused-const-variable=]
> drivers/net/wireless/realtek/rtlwifi/rtl8192d/phy_common.c:91:16: warning: context imbalance in
> 'rtl92d_phy_query_rf_reg' - different lock contexts for basic block
> drivers/net/wireless/realtek/rtlwifi/rtl8192d/phy_common.c:98:6: warning: context imbalance in
> 'rtl92d_phy_set_rf_reg' - different lock contexts for basic block
This looks like sparse is getting confused. I fixed it
by putting both lock and unlock inside the same if,
like above.
> drivers/net/wireless/realtek/rtlwifi/rtl8192d/rf_common.c:8:6: warning: no previous prototype for
> 'rtl92d_phy_rf6052_set_bandwidth' [-Wmissing-prototypes]
> drivers/net/wireless/realtek/rtlwifi/rtl8192d/rf_common.c:46:6: warning: no previous prototype for
> 'rtl92d_phy_rf6052_set_cck_txpower' [-Wmissing-prototypes]
> drivers/net/wireless/realtek/rtlwifi/rtl8192d/rf_common.c:362:6: warning: no previous prototype
> for 'rtl92d_phy_rf6052_set_ofdm_txpower' [-Wmissing-prototypes]
It was missing #include "rf_common.h".
> drivers/net/wireless/realtek/rtlwifi/rtl8192du/phy.c:3055:1: error: expected declaration or
> statement at end of input
> drivers/net/wireless/realtek/rtlwifi/rtl8192du/phy.c:2957:6: warning: 'rtl92du_phy_init_pa_bias'
> defined but not used [-Wunused-function]
> drivers/net/wireless/realtek/rtlwifi/rtl8192du/phy.c:2923:6: warning: 'rtl92d_phy_check_poweroff'
> defined but not used [-Wunused-function]
> drivers/net/wireless/realtek/rtlwifi/rtl8192du/phy.c:2694:6: warning:
> 'rtl92d_update_bbrf_configuration' defined but not used [-Wunused-function]
These warnings seem to be caused by that stray open brace I mentioned.
> drivers/net/wireless/realtek/rtlwifi/rtl8192du/../rtl8192d/phy_common.h:30:17: warning:
> 'channel_all' defined but not used [-Wunused-const-variable=]
> drivers/net/wireless/realtek/rtlwifi/rtl8192d/rf_common.c:8:6: warning: symbol
> 'rtl92d_phy_rf6052_set_bandwidth' was not declared. Should it be static?
> drivers/net/wireless/realtek/rtlwifi/rtl8192d/rf_common.c:46:6: warning: symbol
> 'rtl92d_phy_rf6052_set_cck_txpower' was not declared. Should it be static?
> drivers/net/wireless/realtek/rtlwifi/rtl8192d/rf_common.c:362:6: warning: symbol
> 'rtl92d_phy_rf6052_set_ofdm_txpower' was not declared. Should it be static?
> drivers/net/wireless/realtek/rtlwifi/rtl8192de/../rtl8192d/phy_common.h:30:17: warning:
> 'channel_all' defined but not used [-Wunused-const-variable=]
>
> Please correct them. I will wait for your v3.
>
> Ping-Ke
>
>
