* [PATCH v2] net: wireless: rtlwifi: rtl8192cu: hw.c: Cleaning up if statement that always evaluates to false @ 2014-06-21 21:00 Rickard Strandqvist 2014-06-21 21:00 ` Rickard Strandqvist 0 siblings, 1 reply; 3+ messages in thread From: Rickard Strandqvist @ 2014-06-21 21:00 UTC (permalink / raw) To: Larry Finger, Chaoming Li Cc: Rickard Strandqvist, John W. Linville, Ben Hutchings, Peter Wu, Mark Cave-Ayland, linux-wireless, netdev, linux-kernel I found a logical error in an if statement that always evaluates to false. This has after same discussion resulted in that we add a macro to handle this. This was partly found using a static code analysis program called cppcheck. Rickard Strandqvist (1): net: wireless: rtlwifi: rtl8192cu: hw.c: Cleaning up if statement that always evaluates to false drivers/net/wireless/rtlwifi/rtl8192ce/hw.c | 2 +- drivers/net/wireless/rtlwifi/rtl8192ce/reg.h | 1 + drivers/net/wireless/rtlwifi/rtl8192cu/hw.c | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) -- 1.7.10.4 ^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH v2] net: wireless: rtlwifi: rtl8192cu: hw.c: Cleaning up if statement that always evaluates to false 2014-06-21 21:00 [PATCH v2] net: wireless: rtlwifi: rtl8192cu: hw.c: Cleaning up if statement that always evaluates to false Rickard Strandqvist @ 2014-06-21 21:00 ` Rickard Strandqvist 2014-06-21 21:34 ` Peter Wu 0 siblings, 1 reply; 3+ messages in thread From: Rickard Strandqvist @ 2014-06-21 21:00 UTC (permalink / raw) To: Larry Finger, Chaoming Li Cc: Rickard Strandqvist, John W. Linville, Ben Hutchings, Peter Wu, Mark Cave-Ayland, linux-wireless, netdev, linux-kernel I found a logical error in an if statement that always evaluates to false. This has after same discussion resulted in that we add a macro to handle this. Signed-off-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se> --- drivers/net/wireless/rtlwifi/rtl8192ce/hw.c | 2 +- drivers/net/wireless/rtlwifi/rtl8192ce/reg.h | 1 + drivers/net/wireless/rtlwifi/rtl8192cu/hw.c | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/net/wireless/rtlwifi/rtl8192ce/hw.c b/drivers/net/wireless/rtlwifi/rtl8192ce/hw.c index 55adf04..5050938 100644 --- a/drivers/net/wireless/rtlwifi/rtl8192ce/hw.c +++ b/drivers/net/wireless/rtlwifi/rtl8192ce/hw.c @@ -1206,7 +1206,7 @@ static int _rtl92ce_set_media_status(struct ieee80211_hw *hw, rtl_write_byte(rtlpriv, (MSR), bt_msr); rtlpriv->cfg->ops->led_control(hw, ledaction); - if ((bt_msr & 0xfc) == MSR_AP) + if ((bt_msr & MSR_MASK) == MSR_AP) rtl_write_byte(rtlpriv, REG_BCNTCFG + 1, 0x00); else rtl_write_byte(rtlpriv, REG_BCNTCFG + 1, 0x66); diff --git a/drivers/net/wireless/rtlwifi/rtl8192ce/reg.h b/drivers/net/wireless/rtlwifi/rtl8192ce/reg.h index ed703a1..dc8460c 100644 --- a/drivers/net/wireless/rtlwifi/rtl8192ce/reg.h +++ b/drivers/net/wireless/rtlwifi/rtl8192ce/reg.h @@ -375,6 +375,7 @@ #define MSR_ADHOC 0x01 #define MSR_INFRA 0x02 #define MSR_AP 0x03 +#define MSR_MASK 0x03 #define RRSR_RSC_OFFSET 21 #define RRSR_SHORT_OFFSET 23 diff --git a/drivers/net/wireless/rtlwifi/rtl8192cu/hw.c b/drivers/net/wireless/rtlwifi/rtl8192cu/hw.c index 07cb06d..87c0a27 100644 --- a/drivers/net/wireless/rtlwifi/rtl8192cu/hw.c +++ b/drivers/net/wireless/rtlwifi/rtl8192cu/hw.c @@ -1360,7 +1360,7 @@ static int _rtl92cu_set_media_status(struct ieee80211_hw *hw, } rtl_write_byte(rtlpriv, (MSR), bt_msr); rtlpriv->cfg->ops->led_control(hw, ledaction); - if ((bt_msr & 0xfc) == MSR_AP) + if ((bt_msr & MSR_MASK) == MSR_AP) rtl_write_byte(rtlpriv, REG_BCNTCFG + 1, 0x00); else rtl_write_byte(rtlpriv, REG_BCNTCFG + 1, 0x66); -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] net: wireless: rtlwifi: rtl8192cu: hw.c: Cleaning up if statement that always evaluates to false 2014-06-21 21:00 ` Rickard Strandqvist @ 2014-06-21 21:34 ` Peter Wu 0 siblings, 0 replies; 3+ messages in thread From: Peter Wu @ 2014-06-21 21:34 UTC (permalink / raw) To: Rickard Strandqvist Cc: Larry Finger, Chaoming Li, John W. Linville, Ben Hutchings, Mark Cave-Ayland, linux-wireless, netdev, linux-kernel On Saturday 21 June 2014 23:00:37 Rickard Strandqvist wrote: > I found a logical error in an if statement that always evaluates to false. > > This has after same discussion resulted in that we add a macro to handle this. The subject is still wrong (it is not a cleanup, but a fix) and the commit message could also be improved (no need to mention the discussion, just say why it was necessary and what you did to fix it). Suggested text: rtlwifi/rtl8192c[eu]: fix media status register mask bt_msr & 0xfc will never match 0x3. Fix this by using a mask that actually matches the available types. With that, you can add my Reviewed-by: Peter Wu <peter@lekensteyn.nl> (oh, and please mail me at @lekensteyn.nl for patches) Also, when using a cover letter, you are supposed to group your changes. Not add a single commit to each cover letter. If you made three sequential changes for example, then you can generate your patches with: mkdir /tmp/patches git format-patch --cover-letter -o /tmp/patches/ HEAD~3 Then review the files in /tmp/patches/ and if you are satisfied: git send-email --to linux-wireless@... --cc ... /tmp/patches/* Kind regards, Peter > Signed-off-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se> > --- > drivers/net/wireless/rtlwifi/rtl8192ce/hw.c | 2 +- > drivers/net/wireless/rtlwifi/rtl8192ce/reg.h | 1 + > drivers/net/wireless/rtlwifi/rtl8192cu/hw.c | 2 +- > 3 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/wireless/rtlwifi/rtl8192ce/hw.c b/drivers/net/wireless/rtlwifi/rtl8192ce/hw.c > index 55adf04..5050938 100644 > --- a/drivers/net/wireless/rtlwifi/rtl8192ce/hw.c > +++ b/drivers/net/wireless/rtlwifi/rtl8192ce/hw.c > @@ -1206,7 +1206,7 @@ static int _rtl92ce_set_media_status(struct ieee80211_hw *hw, > > rtl_write_byte(rtlpriv, (MSR), bt_msr); > rtlpriv->cfg->ops->led_control(hw, ledaction); > - if ((bt_msr & 0xfc) == MSR_AP) > + if ((bt_msr & MSR_MASK) == MSR_AP) > rtl_write_byte(rtlpriv, REG_BCNTCFG + 1, 0x00); > else > rtl_write_byte(rtlpriv, REG_BCNTCFG + 1, 0x66); > diff --git a/drivers/net/wireless/rtlwifi/rtl8192ce/reg.h b/drivers/net/wireless/rtlwifi/rtl8192ce/reg.h > index ed703a1..dc8460c 100644 > --- a/drivers/net/wireless/rtlwifi/rtl8192ce/reg.h > +++ b/drivers/net/wireless/rtlwifi/rtl8192ce/reg.h > @@ -375,6 +375,7 @@ > #define MSR_ADHOC 0x01 > #define MSR_INFRA 0x02 > #define MSR_AP 0x03 > +#define MSR_MASK 0x03 > > #define RRSR_RSC_OFFSET 21 > #define RRSR_SHORT_OFFSET 23 > diff --git a/drivers/net/wireless/rtlwifi/rtl8192cu/hw.c b/drivers/net/wireless/rtlwifi/rtl8192cu/hw.c > index 07cb06d..87c0a27 100644 > --- a/drivers/net/wireless/rtlwifi/rtl8192cu/hw.c > +++ b/drivers/net/wireless/rtlwifi/rtl8192cu/hw.c > @@ -1360,7 +1360,7 @@ static int _rtl92cu_set_media_status(struct ieee80211_hw *hw, > } > rtl_write_byte(rtlpriv, (MSR), bt_msr); > rtlpriv->cfg->ops->led_control(hw, ledaction); > - if ((bt_msr & 0xfc) == MSR_AP) > + if ((bt_msr & MSR_MASK) == MSR_AP) > rtl_write_byte(rtlpriv, REG_BCNTCFG + 1, 0x00); > else > rtl_write_byte(rtlpriv, REG_BCNTCFG + 1, 0x66); > ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-06-21 21:34 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-06-21 21:00 [PATCH v2] net: wireless: rtlwifi: rtl8192cu: hw.c: Cleaning up if statement that always evaluates to false Rickard Strandqvist 2014-06-21 21:00 ` Rickard Strandqvist 2014-06-21 21:34 ` Peter Wu
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).