On Tue, 2018-08-07 at 09:23 -0500, Larry Finger wrote:
> On 08/06/2018 04:42 PM, valdis.kletnieks@xxxxxx 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@xxxxxxxxxxxx>
> > 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.
>
>
