All of lore.kernel.org
 help / color / mirror / Atom feed
From: Joe Perches <joe@perches.com>
To: Larry Finger <Larry.Finger@lwfinger.net>, 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 14:23:16 -0700	[thread overview]
Message-ID: <1467062596.24287.4.camel@perches.com> (raw)
In-Reply-To: <1467042758-25742-11-git-send-email-Larry.Finger@lwfinger.net>

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?

> +			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?

> +				}
> +				break;
> +			}
> +		case 0x8178:
> +			switch (rtlefuse->eeprom_svid) {
> +			case 0x10ec:
> +				switch (rtlefuse->eeprom_smid) {
> +				case 0x6181 ... 0x6182:
> +				case 0x6184 ... 0x6185:
> +				case 0x7181 ... 0x7182:
> +				case 0x7184 ... 0x7185:
> +				case 0x8181 ... 0x8182:
> +				case 0x8184 ... 0x8185:
> +				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, 0x8186))
> -					rtlhal->oem_id = RT_CID_819X_PRONETS;
> -				else if (CHK_SVID_SMID(0x1043, 0x8486))
> +					break;
> +				case 0x8186:
>  					rtlhal->oem_id =
> -						     RT_CID_819X_EDIMAX_ASUS;
> -				else
> -					rtlhal->oem_id = RT_CID_DEFAULT;
> -			} else {
> -				rtlhal->oem_id = RT_CID_DEFAULT;
> -			}
> -			break;
> -		case EEPROM_CID_TOSHIBA:
> -			rtlhal->oem_id = RT_CID_TOSHIBA;
> -			break;
> -		case EEPROM_CID_CCX:
> -			rtlhal->oem_id = RT_CID_CCX;
> -			break;
> -		case EEPROM_CID_QMI:
> -			rtlhal->oem_id = RT_CID_819X_QMI;
> -			break;
> -		case EEPROM_CID_WHQL:
> +						RT_CID_819X_PRONETS;
> +					break;
> +				}
>  				break;
> -		default:
> -			rtlhal->oem_id = RT_CID_DEFAULT;
> +			case 0x1025:
> +				rtlhal->oem_id = RT_CID_819X_ACER;
> +				break;
> +			case 0x1043:
> +				switch (rtlefuse->eeprom_smid) {
> +				case 0x8486:
> +					rtlhal->oem_id =
> +					     RT_CID_819X_EDIMAX_ASUS;
> +				}
> +				break;
> +			}
>  			break;
> -
>  		}
> +	case EEPROM_CID_TOSHIBA:
> +		rtlhal->oem_id = RT_CID_TOSHIBA;
> +		break;
> +	case EEPROM_CID_CCX:
> +		rtlhal->oem_id = RT_CID_CCX;
> +		break;
> +	case EEPROM_CID_QMI:
> +		rtlhal->oem_id = RT_CID_819X_QMI;
> +		break;
> +	case EEPROM_CID_WHQL:
> +		break;
> +	default:
> +		rtlhal->oem_id = RT_CID_DEFAULT;
> +		break;
> +
>  	}
>  exit:
>  	kfree(hwinfo);

  reply	other threads:[~2016-06-27 21:23 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 [this message]
2016-06-28  0:56     ` Larry Finger

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=1467062596.24287.4.camel@perches.com \
    --to=joe@perches.com \
    --cc=Larry.Finger@lwfinger.net \
    --cc=devel@driverdev.osuosl.org \
    --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.