From: Pkshih <pkshih@realtek.com>
To: "yuehaibing@huawei.com" <yuehaibing@huawei.com>,
"valdis.kletnieks@vt.edu" <valdis.kletnieks@vt.edu>,
"Larry.Finger@lwfinger.net" <Larry.Finger@lwfinger.net>
Cc: "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
"kvalo@codeaurora.org" <kvalo@codeaurora.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"colin.king@canonical.com" <colin.king@canonical.com>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
"davem@davemloft.net" <davem@davemloft.net>
Subject: Re: [PATCH] rtlwifi: btcoex: Fix if == else warnings in halbtc8723b2ant.c
Date: Wed, 8 Aug 2018 01:48:14 +0000 [thread overview]
Message-ID: <1533692892.2513.16.camel@realtek.com> (raw)
In-Reply-To: <577a831f-45c3-a687-ca04-edc977ddb925@lwfinger.net>
T24gVHVlLCAyMDE4LTA4LTA3IGF0IDA5OjIzIC0wNTAwLCBMYXJyeSBGaW5nZXIgd3JvdGU6DQo+
IE9uIDA4LzA2LzIwMTggMDQ6NDIgUE0sIHZhbGRpcy5rbGV0bmlla3NAdnQuZWR1IHdyb3RlOg0K
PiA+IE9uIE1vbiwgMDYgQXVnIDIwMTggMTI6NTQ6NDAgKzA4MDAsIFl1ZUhhaWJpbmcgc2FpZDoN
Cj4gPj4gRml4IGZvbGxvd2luZyBjb2NjaW5lbGxlIHdhcm5pbmc6DQo+ID4+DQo+ID4+IC4vZHJp
dmVycy9uZXQvd2lyZWxlc3MvcmVhbHRlay9ydGx3aWZpL2J0Y29leGlzdC9oYWxidGM4NzIzYjJh
bnQuYzoyOTUyOjItNDogV0FSTklORzogcG9zc2libGUNCj4gY29uZGl0aW9uIHdpdGggbm8gZWZm
ZWN0IChpZiA9PSBlbHNlKQ0KPiA+wqANCj4gPj7CoMKgwqAJLyogc3cgbWVjaGFuaXNtICovDQo+
ID4+wqDCoMKgCWlmIChCVENfV0lGSV9CV19IVDQwID09IHdpZmlfYncpIHsNCj4gPj4gLQkJaWYg
KCh3aWZpX3Jzc2lfc3RhdGUgPT0gQlRDX1JTU0lfU1RBVEVfSElHSCkgfHwNCj4gPj4gLQkJwqDC
oMKgwqAod2lmaV9yc3NpX3N0YXRlID09IEJUQ19SU1NJX1NUQVRFX1NUQVlfSElHSCkpIHsNCj4g
Pj4gLQkJCWJ0Yzg3MjNiMmFudF9zd19tZWNoYW5pc20oYnRjb2V4aXN0LCB0cnVlLCB0cnVlLA0K
PiA+PiAtCQkJCQkJwqDCoGZhbHNlLCBmYWxzZSk7DQo+ID4+IC0JCX0gZWxzZSB7DQo+ID4+IC0J
CQlidGM4NzIzYjJhbnRfc3dfbWVjaGFuaXNtKGJ0Y29leGlzdCwgdHJ1ZSwgdHJ1ZSwNCj4gPj4g
LQkJCQkJCcKgwqBmYWxzZSwgZmFsc2UpOw0KPiA+PiAtCQl9DQo+ID4+ICsJCWJ0Yzg3MjNiMmFu
dF9zd19tZWNoYW5pc20oYnRjb2V4aXN0LCB0cnVlLCB0cnVlLA0KPiA+PiArCQkJCQnCoMKgZmFs
c2UsIGZhbHNlKTsNCj4gPj7CoMKgwqAJfSBlbHNlIHsNCj4gPsKgDQo+ID4gUmF0aGVyIHRoYW4g
YmxpbmRseSBmaXhpbmcgdGhpcywgcGVyaGFwcyBhIGJpdCBvZiB0aG91Z2h0IG5lZWRzIHRvIGJl
DQo+ID4gYXBwbGllZCB0byB3aHkgdGhpcyBjb2RlIGxvb2tzIGxpa2UgdGhpcyBpbiB0aGUgZmly
c3QgcGxhY2UuDQo+ID7CoA0KPiA+IFNlZSBjb21taXQgYzY4MjE2MTNlNjUzYcKgwqAod2hpY2gg
bG9va3MgbGlrZSB0aGUgYmxldGNoZXJvdXMgImRvIHRvbyBtYW55DQo+ID4gdGhpbmdzIGF0IG9u
Y2UiIGNvbW1pdCBpbmRlZWQpLCBhbHRob3VnaCB0aGUgYWN0dWFsIGRpZmYgYXBwZWFycyB0byBi
ZSBhDQo+ID4gIm5vIGhhcm0sIG5vIGZvdWwiIGFnYWluc3QgdGhpcyBjb21taXQsIHdoZXJlIHRo
ZSBpc3N1ZSBhbHJlYWR5IGV4aXN0ZWQuDQo+ID7CoA0KPiA+IGNvbW1pdCBhYTQ1YTY3M2IyOTFm
ZDc2MTI3NTQ5M2JjMTUzMTZkNzk1NTVlZDU1DQo+ID4gQXV0aG9yOiBMYXJyeSBGaW5nZXIgPExh
cnJ5LkZpbmdlckBsd2Zpbmdlci5uZXQ+DQo+ID4gRGF0ZTrCoMKgwqBGcmkgRmViIDI4IDE1OjE2
OjQzIDIwMTQgLTA2MDANCj4gPsKgDQo+ID7CoMKgwqDCoMKgwqBydGx3aWZpOiBidGNvZXhpc3Q6
IEFkZCBuZXcgbWluaSBkcml2ZXINCj4gPsKgDQo+ID4gTGFycnk/IENhbiB5b3UgcmVhY2ggYmFj
ayB0byAyMDE0IGFuZCByZW1lbWJlciB3aHkgdGhpcyBjb2RlDQo+ID4gbG9va2VkIGxpa2UgdGhp
cyBpbiB0aGUgZmlyc3QgcGxhY2U/DQo+IA0KPiBUaGUgYmFzZSBjb2RlIGNhbWUgZnJvbSBSZWFs
dGVrLiBNeSBvbmx5IHBhcnQgaW4gZ2V0dGluZyBpdCBpbnRvIHRoZSBrZXJuZWwgd2FzwqANCj4g
dG8gY2xlYW4gdXAgdGhlIGNoZWNrcGF0Y2ggYW5kIFNwYXJzZSBlcnJvcnMgYW5kIHdhcm5pbmdz
LCBhbmQgc3VibWl0IGl0LiBJIHdhc8KgDQo+IGEgInNjcmlwdCBraWRkeSIganVzdCBsaWtlIHRo
ZSBhdXRob3JzIG9mIHRoZSBjdXJyZW50IHBhdGNoZXMuIFRoZSBvbmx5wqANCj4gZGlmZmVyZW5j
ZSBpcyB0aGF0IEkgd2FzIGdldHRpbmcgZHJpdmVycyBpbnRvIHRoZSBrZXJuZWwgc28gdGhhdCB1
c2VycyBoYXJkd2FyZcKgDQo+IHdvdWxkIHdvcmsuDQo+IA0KPiBBbnkgcmVjb3JkIG9mIHdoZXRo
ZXIgdGhlc2UgZHVwbGljYXRlIGJyYW5jaGVzIGFyZSB0aGUgcmVzdWx0IG9mIGluY29ycmVjdCBj
b3B5wqANCj4gYW5kIHBhc3RlLCBvciBqdXN0IGV4dHJhbmVvdXMgY29kZSwgd291bGQgYmUgYXQg
UmVhbHRlayBpbiB0aGVpciB2ZXJzaW9uIGNvbnRyb2zCoA0KPiBoaXN0b3J5LiBJIGhhdmUgbmV2
ZXIgaGFkIGFjY2VzcyB0byBzdWNoIGFyY2hpdmVzLCBub3IgaGF2ZSBJIGV2ZXIgaGFkIGFuwqAN
Cj4gbm9uLWRpc2Nsb3N1cmUgYWdyZWVtZW50IHdpdGggUmVhbHRlay4NCj4gDQo+IFBpbmctS2Ug
U2hpaCwgd2hvIGlzIENjJ2Qgb24gdGhlc2UgbWVzc2FnZXMsIG1pZ2h0IGJlIGFibGUgdG8gYW5z
d2VyIHRoZXNlwqANCj4gcXVlc3Rpb25zLg0KDQpUaGVzZSBicmFuY2hlcyBpcyB1c2VkIHRvIGlt
cHJvdmUgdXNlciBleHBlcmllbmNlIGFjY29yZGluZyB0byBSU1NJIHN0cmVuZ3RoLCBidXQNCml0
IGhhcyBub3Qgc2lnbmlmaWNhbnQgaW1wcm92ZW1lbnQgc28gdGhlIHNhbWUgYXJndW1lbnRzIGJl
Y29tZSBkdXBsaWNhdGUgYnJhbmNoZXMuDQpJIGNoZWNrIHRoZSBsYXRlc3QgY29kZSBvZsKgaGFs
YnRjODcyM2IyYW50LmMsIHRoZXJlIGFyZSBtb3JlIHRoYW4gb25lIHN0YXRlbWVudHMNCndpdGhp
biBpZi1lbHNlIGJyYW5jaCB0byBpbXByb3ZlIHBlcmZvcm1hbmNlLCBidXQgdGhlIHN0YXRlbWVu
dHMgbWVudGlvbmVkIGJ5wqANCnRoaXMgcGF0Y2ggYXJlIHN0aWxsIHRoZSBzYW1lLiBTbywgdGhl
c2UgZHVwbGljYXRlIGJyYW5jaGVzIGNhbiBiZSBzYWZlbHkgcmVtb3ZlZC4NCg0KPiANCj4gRm9y
IHRoZSBtb21lbnQsIHRoZXNlIHNpbXBsaWZpY2F0aW9ucyBjb3VsZCBiZSBhcHBsaWVkIGFzIGxv
bmcgYXMgdGhleSBhcmXCoA0KPiBjb3JyZWN0bHkgZG9uZS4gQWZ0ZXIgYWxsLCB0aGV5IGFyZSBu
b3QgY2hhbmdpbmcgd2hhdCBpcyBhbHJlYWR5IHRoZXJlIGFuZCB0aGF0wqANCj4gc3RvcHMgYW55
IG90aGVyIHBlcnNvbiB0aGF0IGtub3dzIGhvdyB0byBydW4gY29jY2luZWxsZSBmcm9tIHN1Ym1p
dHRpbmcgdGhlIHNhbWXCoA0KPiBwYXRjaCBpbiB0aGUgZnV0dXJlLiBXaHkgImtpY2sgdGhlIGNh
biBkb3duIHRoZSByb2FkIj8gSWYgUEsgY2FuIGZpbmQgdGhhdCB0aGVyZcKgDQo+IHdhcyBhbiBl
cnJvciBpbiB0aGUgb3JpZ2luYWwgY29kZSwgaGUgY2FuIHN1Ym1pdCBhICJGaXhlcyIgcGF0Y2gu
DQo+IA0KPiANCg0K
WARNING: multiple messages have this Message-ID (diff)
From: Pkshih <pkshih@realtek.com>
To: "yuehaibing@huawei.com" <yuehaibing@huawei.com>,
"valdis.kletnieks@vt.edu" <valdis.kletnieks@vt.edu>,
"Larry.Finger@lwfinger.net" <Larry.Finger@lwfinger.net>
Cc: "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
"kvalo@codeaurora.org" <kvalo@codeaurora.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"colin.king@canonical.com" <colin.king@canonical.com>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
"davem@davemloft.net" <davem@davemloft.net>
Subject: Re: [PATCH] rtlwifi: btcoex: Fix if == else warnings in halbtc8723b2ant.c
Date: Wed, 8 Aug 2018 01:48:14 +0000 [thread overview]
Message-ID: <1533692892.2513.16.camel@realtek.com> (raw)
In-Reply-To: <577a831f-45c3-a687-ca04-edc977ddb925@lwfinger.net>
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.
>
>
next prev parent reply other threads:[~2018-08-08 4:05 UTC|newest]
Thread overview: 9+ 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 6:13 ` Pkshih
2018-08-06 21:42 ` valdis.kletnieks
2018-08-06 21:42 ` valdis.kletnieks-PjAqaU27lzQ
2018-08-07 14:23 ` Larry Finger
2018-08-08 1:48 ` Pkshih [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=1533692892.2513.16.camel@realtek.com \
--to=pkshih@realtek.com \
--cc=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=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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.