On 20/03/2024 02:57, Ping-Ke Shih wrote:
>
>>> 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;
>> }
>>
>
> Duplicate of main statements 'temp_cck = ....' isn't good. I prefer
>
> bool need_lock = rtlpriv->rtlhal.interfaceindex == 1 &&
> rtlpriv->rtlhal.interface == INTF_PCI;
>
> if (need_lock)
> spin_lock_irqsave(&rtlpriv->locks.cck_and_rw_pagea_lock, flag);
>
> temp_cck = rtl_get_bbreg(hw, RCCK0_TXFILTER2, MASKDWORD) & MASKCCK;
>
> if (need_lock)
> spin_unlock_irqrestore(&rtlpriv->locks.cck_and_rw_pagea_lock, flag);
>
Even this doesn't work. I get the same warning as before.
>
> But, I wonder why sparse doesn't complain original code (before your patchset)
> that used static inline already. Can we keep original style?
>
I found the reason. In patch 2/12 I moved the two functions
from rtl8192de/phy.h to rtl8192d/phy_common.h. This should be
harmless. But I also deleted these lines from the end of
rtl8192de/phy.h:
void rtl92d_acquire_cckandrw_pagea_ctl(struct ieee80211_hw *hw,
unsigned long *flag);
void rtl92d_release_cckandrw_pagea_ctl(struct ieee80211_hw *hw,
unsigned long *flag);
They seemed pointless. If I add them to phy_common.h all the
warnings about locks go away. I will do this for v3.
