lkml.org 
[lkml]   [2018]   [Aug]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH] rtlwifi: btcoex: Fix if == else warnings in halbtc8723b2ant.c
Date
On Tue, 2018-08-07 at 09:23 -0500, Larry Finger wrote:
> On 08/06/2018 04:42 PM, valdis.kletnieks@vt.edu wrote:
> > On Mon, 06 Aug 2018 12:54:40 +0800, YueHaibing said:
> >> Fix following coccinelle warning:
> >>
> >> ./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b2ant.c:2952:2-4: WARNING: possible
> condition with no effect (if == else)
> > 
> >>    /* sw mechanism */
> >>    if (BTC_WIFI_BW_HT40 == wifi_bw) {
> >> - if ((wifi_rssi_state == BTC_RSSI_STATE_HIGH) ||
> >> -     (wifi_rssi_state == BTC_RSSI_STATE_STAY_HIGH)) {
> >> - btc8723b2ant_sw_mechanism(btcoexist, true, true,
> >> -   false, false);
> >> - } else {
> >> - btc8723b2ant_sw_mechanism(btcoexist, true, true,
> >> -   false, false);
> >> - }
> >> + btc8723b2ant_sw_mechanism(btcoexist, true, true,
> >> +   false, false);
> >>    } else {
> > 
> > Rather than blindly fixing this, perhaps a bit of thought needs to be
> > applied to why this code looks like this in the first place.
> > 
> > See commit c6821613e653a  (which looks like the bletcherous "do too many
> > things at once" commit indeed), although the actual diff appears to be a
> > "no harm, no foul" against this commit, where the issue already existed.
> > 
> > commit aa45a673b291fd761275493bc15316d79555ed55
> > Author: Larry Finger <Larry.Finger@lwfinger.net>
> > Date:   Fri Feb 28 15:16:43 2014 -0600
> > 
> >      rtlwifi: btcoexist: Add new mini driver
> > 
> > Larry? Can you reach back to 2014 and remember why this code
> > looked like this in the first place?
>
> The base code came from Realtek. My only part in getting it into the kernel was 
> to clean up the checkpatch and Sparse errors and warnings, and submit it. I was 
> a "script kiddy" just like the authors of the current patches. The only 
> difference is that I was getting drivers into the kernel so that users hardware 
> would work.
>
> Any record of whether these duplicate branches are the result of incorrect copy 
> and paste, or just extraneous code, would be at Realtek in their version control 
> history. I have never had access to such archives, nor have I ever had an 
> non-disclosure agreement with Realtek.
>
> Ping-Ke Shih, who is Cc'd on these messages, might be able to answer these 
> questions.

These branches is used to improve user experience according to RSSI strength, but
it has not significant improvement so the same arguments become duplicate branches.
I check the latest code of halbtc8723b2ant.c, there are more than one statements
within if-else branch to improve performance, but the statements mentioned by 
this patch are still the same. So, these duplicate branches can be safely removed.

>
> For the moment, these simplifications could be applied as long as they are 
> correctly done. After all, they are not changing what is already there and that 
> stops any other person that knows how to run coccinelle from submitting the same 
> patch in the future. Why "kick the can down the road"? If PK can find that there 
> was an error in the original code, he can submit a "Fixes" patch.
>
>

\
 
 \ /
  Last update: 2018-08-08 03:49    [W:0.077 / U:0.532 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site