From: Larry Finger <Larry.Finger@lwfinger.net>
To: valdis.kletnieks@vt.edu, YueHaibing <yuehaibing@huawei.com>
Cc: pkshih@realtek.com, kvalo@codeaurora.org,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
davem@davemloft.net, colin.king@canonical.com,
linux-wireless@vger.kernel.org
Subject: Re: [PATCH] rtlwifi: btcoex: Fix if == else warnings in halbtc8723b2ant.c
Date: Tue, 7 Aug 2018 09:23:30 -0500 [thread overview]
Message-ID: <577a831f-45c3-a687-ca04-edc977ddb925@lwfinger.net> (raw)
In-Reply-To: <63216.1533591775@turing-police.cc.vt.edu>
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.
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.
Larry
next prev parent reply other threads:[~2018-08-07 14:23 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-08-06 4:54 [PATCH] rtlwifi: btcoex: Fix if == else warnings in halbtc8723b2ant.c YueHaibing
2018-08-06 6:13 ` Pkshih
2018-08-06 21:42 ` valdis.kletnieks
2018-08-07 14:23 ` Larry Finger [this message]
2018-08-08 1:48 ` Pkshih
2018-08-09 15:15 ` Kalle Valo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=577a831f-45c3-a687-ca04-edc977ddb925@lwfinger.net \
--to=larry.finger@lwfinger.net \
--cc=colin.king@canonical.com \
--cc=davem@davemloft.net \
--cc=kvalo@codeaurora.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-wireless@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pkshih@realtek.com \
--cc=valdis.kletnieks@vt.edu \
--cc=yuehaibing@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).