From: Larry Finger <Larry.Finger@lwfinger.net>
To: Joe Perches <joe@perches.com>, kvalo@codeaurora.org
Cc: devel@driverdev.osuosl.org, linux-wireless@vger.kernel.org
Subject: Re: [PATCH v2 10/10] rtlwifi: rtl8723ae: Clean up the hardware info routine
Date: Mon, 27 Jun 2016 19:56:52 -0500 [thread overview]
Message-ID: <5771CB54.2010307@lwfinger.net> (raw)
In-Reply-To: <1467062596.24287.4.camel@perches.com>
On 06/27/2016 04:23 PM, Joe Perches wrote:
> On Mon, 2016-06-27 at 10:52 -0500, Larry Finger wrote:
>> This driver contains some complicated if ... else if ... else
>> constructions. These are replaced by switch statements to improve
>> readability.
> []
>> diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8723ae/hw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8723ae/hw.c
> []
>> @@ -1653,132 +1653,134 @@ static void _rtl8723e_read_adapter_info(struct ieee80211_hw *hw,
>> rtl8723e_read_bt_coexist_info_from_hwpg(hw,
>> rtlefuse->autoload_failflag, hwinfo);
>>
>> - if (rtlhal->oem_id == RT_CID_DEFAULT) {
>> - switch (rtlefuse->eeprom_oemid) {
>> - case EEPROM_CID_DEFAULT:
>> - if (rtlefuse->eeprom_did == 0x8176) {
>> - if (CHK_SVID_SMID(0x10EC, 0x6151) ||
>> - CHK_SVID_SMID(0x10EC, 0x6152) ||
>> - CHK_SVID_SMID(0x10EC, 0x6154) ||
>> - CHK_SVID_SMID(0x10EC, 0x6155) ||
>> - CHK_SVID_SMID(0x10EC, 0x6177) ||
>> - CHK_SVID_SMID(0x10EC, 0x6178) ||
>> - CHK_SVID_SMID(0x10EC, 0x6179) ||
>> - CHK_SVID_SMID(0x10EC, 0x6180) ||
>> - CHK_SVID_SMID(0x10EC, 0x7151) ||
>> - CHK_SVID_SMID(0x10EC, 0x7152) ||
>> - CHK_SVID_SMID(0x10EC, 0x7154) ||
>> - CHK_SVID_SMID(0x10EC, 0x7155) ||
>> - CHK_SVID_SMID(0x10EC, 0x7177) ||
>> - CHK_SVID_SMID(0x10EC, 0x7178) ||
>> - CHK_SVID_SMID(0x10EC, 0x7179) ||
>> - CHK_SVID_SMID(0x10EC, 0x7180) ||
>> - CHK_SVID_SMID(0x10EC, 0x8151) ||
>> - CHK_SVID_SMID(0x10EC, 0x8152) ||
>> - CHK_SVID_SMID(0x10EC, 0x8154) ||
>> - CHK_SVID_SMID(0x10EC, 0x8155) ||
>> - CHK_SVID_SMID(0x10EC, 0x8181) ||
>> - CHK_SVID_SMID(0x10EC, 0x8182) ||
>> - CHK_SVID_SMID(0x10EC, 0x8184) ||
>> - CHK_SVID_SMID(0x10EC, 0x8185) ||
>> - CHK_SVID_SMID(0x10EC, 0x9151) ||
>> - CHK_SVID_SMID(0x10EC, 0x9152) ||
>> - CHK_SVID_SMID(0x10EC, 0x9154) ||
>> - CHK_SVID_SMID(0x10EC, 0x9155) ||
>> - CHK_SVID_SMID(0x10EC, 0x9181) ||
>> - CHK_SVID_SMID(0x10EC, 0x9182) ||
>> - CHK_SVID_SMID(0x10EC, 0x9184) ||
>> - CHK_SVID_SMID(0x10EC, 0x9185))
>> + if (rtlhal->oem_id != RT_CID_DEFAULT)
>> + return;
>> +
>> + switch (rtlefuse->eeprom_oemid) {
>> + case EEPROM_CID_DEFAULT:
>> + switch (rtlefuse->eeprom_did) {
>> + case 0x8176:
>> + switch (rtlefuse->eeprom_svid) {
>> + case 0x10EC:
>> + switch (rtlefuse->eeprom_smid) {
>> + case 0x6151 ... 0x6152:
>> + case 0x6154 ... 0x6155:
>> + case 0x6177 ... 0x6180:
>> + case 0x7151 ... 0x7152:
>> + case 0x7154 ... 0x7155:
>> + case 0x7177 ... 0x7180:
>> + case 0x8151 ... 0x8152:
>> + case 0x8154 ... 0x8155:
>> + case 0x8181 ... 0x8182:
>> + case 0x8184 ... 0x8185:
>> + case 0x9151 ... 0x9152:
>> + case 0x9154 ... 0x9155:
>> + case 0x9181 ... 0x9182:
>> + case 0x9184 ... 0x9185:
>> rtlhal->oem_id = RT_CID_TOSHIBA;
>> - else if (rtlefuse->eeprom_svid == 0x1025)
>> - rtlhal->oem_id = RT_CID_819X_ACER;
>> - else if (CHK_SVID_SMID(0x10EC, 0x6191) ||
>> - CHK_SVID_SMID(0x10EC, 0x6192) ||
>> - CHK_SVID_SMID(0x10EC, 0x6193) ||
>> - CHK_SVID_SMID(0x10EC, 0x7191) ||
>> - CHK_SVID_SMID(0x10EC, 0x7192) ||
>> - CHK_SVID_SMID(0x10EC, 0x7193) ||
>> - CHK_SVID_SMID(0x10EC, 0x8191) ||
>> - CHK_SVID_SMID(0x10EC, 0x8192) ||
>> - CHK_SVID_SMID(0x10EC, 0x8193) ||
>> - CHK_SVID_SMID(0x10EC, 0x9191) ||
>> - CHK_SVID_SMID(0x10EC, 0x9192) ||
>> - CHK_SVID_SMID(0x10EC, 0x9193))
>> + break;
>> + case 0x6191 ... 0x6193:
>> + case 0x7191 ... 0x7193:
>> + case 0x8191 ... 0x8193:
>> + case 0x9191 ... 0x9193:
>> rtlhal->oem_id = RT_CID_819X_SAMSUNG;
>> - else if (CHK_SVID_SMID(0x10EC, 0x8195) ||
>> - CHK_SVID_SMID(0x10EC, 0x9195) ||
>> - CHK_SVID_SMID(0x10EC, 0x7194) ||
>> - CHK_SVID_SMID(0x10EC, 0x8200) ||
>> - CHK_SVID_SMID(0x10EC, 0x8201) ||
>> - CHK_SVID_SMID(0x10EC, 0x8202) ||
>> - CHK_SVID_SMID(0x10EC, 0x9200))
>> - rtlhal->oem_id = RT_CID_819X_LENOVO;
>> - else if (CHK_SVID_SMID(0x10EC, 0x8197) ||
>> - CHK_SVID_SMID(0x10EC, 0x9196))
>> + break;
>> + case 0x8197:
>> + case 0x9196:
>> rtlhal->oem_id = RT_CID_819X_CLEVO;
>> - else if (CHK_SVID_SMID(0x1028, 0x8194) ||
>> - CHK_SVID_SMID(0x1028, 0x8198) ||
>> - CHK_SVID_SMID(0x1028, 0x9197) ||
>> - CHK_SVID_SMID(0x1028, 0x9198))
>> + break;
>> + case 0x8203:
>> + rtlhal->oem_id = RT_CID_819X_PRONETS;
>> + break;
>> + case 0x8195:
>> + case 0x9195:
>> + case 0x7194:
>> + case 0x8200 ... 0x8202:
>> + case 0x9200:
>> + rtlhal->oem_id = RT_CID_819X_LENOVO;
>> + break;
>> + }
>
> Is this supposed to be a fallthrough?
> If so, a comment would be good.
> Otherwise is this a missing break?
Good catch. There should be a break here.
>> + case 0x1025:
>> + rtlhal->oem_id = RT_CID_819X_ACER;
>> + break;
>> + case 0x1028:
>> + switch (rtlefuse->eeprom_smid) {
>> + case 0x8194:
>> + case 0x8198:
>> + case 0x9197 ... 0x9198:
>> rtlhal->oem_id = RT_CID_819X_DELL;
>> - else if (CHK_SVID_SMID(0x103C, 0x1629))
>> + break;
>> + }
>> + break;
>> + case 0x103C:
>> + switch (rtlefuse->eeprom_smid) {
>> + case 0x1629:
>> rtlhal->oem_id = RT_CID_819X_HP;
>> - else if (CHK_SVID_SMID(0x1A32, 0x2315))
>> + }
>> + break;
>> + case 0x1A32:
>> + switch (rtlefuse->eeprom_smid) {
>> + case 0x2315:
>> rtlhal->oem_id = RT_CID_819X_QMI;
>> - else if (CHK_SVID_SMID(0x10EC, 0x8203))
>> - rtlhal->oem_id = RT_CID_819X_PRONETS;
>> - else if (CHK_SVID_SMID(0x1043, 0x84B5))
>> + break;
>> + }
>> + break;
>> + case 0x1043:
>> + switch (rtlefuse->eeprom_smid) {
>> + case 0x84B5:
>> rtlhal->oem_id =
>> - RT_CID_819X_EDIMAX_ASUS;
>> - else
>> - rtlhal->oem_id = RT_CID_DEFAULT;
>> - } else if (rtlefuse->eeprom_did == 0x8178) {
>> - if (CHK_SVID_SMID(0x10EC, 0x6181) ||
>> - CHK_SVID_SMID(0x10EC, 0x6182) ||
>> - CHK_SVID_SMID(0x10EC, 0x6184) ||
>> - CHK_SVID_SMID(0x10EC, 0x6185) ||
>> - CHK_SVID_SMID(0x10EC, 0x7181) ||
>> - CHK_SVID_SMID(0x10EC, 0x7182) ||
>> - CHK_SVID_SMID(0x10EC, 0x7184) ||
>> - CHK_SVID_SMID(0x10EC, 0x7185) ||
>> - CHK_SVID_SMID(0x10EC, 0x8181) ||
>> - CHK_SVID_SMID(0x10EC, 0x8182) ||
>> - CHK_SVID_SMID(0x10EC, 0x8184) ||
>> - CHK_SVID_SMID(0x10EC, 0x8185) ||
>> - CHK_SVID_SMID(0x10EC, 0x9181) ||
>> - CHK_SVID_SMID(0x10EC, 0x9182) ||
>> - CHK_SVID_SMID(0x10EC, 0x9184) ||
>> - CHK_SVID_SMID(0x10EC, 0x9185))
>> + RT_CID_819X_EDIMAX_ASUS;
>
> Single line?
Nope - it reaches 81 characters. If that is better than splitting the line, then
I will have to comment it to keep from getting several patches changing it back.
Larry
prev parent reply other threads:[~2016-06-28 0:56 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-06-27 15:52 [PATCH 00/10 V2] rtlwifi: Various clean-ups for the hwinfo routines Larry Finger
2016-06-27 15:52 ` [PATCH v2 01/10] rtlwifi: Create common routine to get hardware info Larry Finger
2016-06-27 21:25 ` Arnd Bergmann
2016-06-27 15:52 ` [PATCH v2 02/10] rtlwifi: rtl8192ce: Convert driver to use common hardware info routine Larry Finger
2016-06-27 15:52 ` [PATCH v2 03/10] rtlwifi: rtl8192cu: " Larry Finger
2016-06-27 15:52 ` [PATCH v2 04/10] rtlwifi: rtl8188ee: " Larry Finger
2016-06-27 15:52 ` [PATCH v2 05/10] rtlwifi: rtl8192ee: " Larry Finger
2016-06-27 15:52 ` [PATCH v2 06/10] rtlwifi: rtl8723ae: " Larry Finger
2016-06-27 15:52 ` [PATCH v2 07/10] rtlwifi: rtl8723be: " Larry Finger
2016-06-27 15:52 ` [PATCH v2 08/10] rtlwifi: rtl8821ae: " Larry Finger
2016-06-27 15:52 ` [PATCH v2 09/10] rtlwifi: rtl8192de: " Larry Finger
2016-06-27 15:52 ` [PATCH v2 10/10] rtlwifi: rtl8723ae: Clean up the " Larry Finger
2016-06-27 21:23 ` Joe Perches
2016-06-28 0:56 ` Larry Finger [this message]
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=5771CB54.2010307@lwfinger.net \
--to=larry.finger@lwfinger.net \
--cc=devel@driverdev.osuosl.org \
--cc=joe@perches.com \
--cc=kvalo@codeaurora.org \
--cc=linux-wireless@vger.kernel.org \
/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.