diff mbox

staging:rtl8723au: core: Fix error reported by checkpatch.

Message ID 1414396745-8845-1-git-send-email-sanjeev_sharma@mentor.com (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Sharma, Sanjeev Oct. 27, 2014, 7:59 a.m. UTC
This is a patch to the rtw_cmd.c file that fixes
Error reported by checkpatch.

Signed-off-by: Sanjeev Sharma <Sanjeev_Sharma@mentor.com>
---
 drivers/staging/rtl8723au/core/rtw_cmd.c | 83 +++++++++++++++-----------------
 1 file changed, 40 insertions(+), 43 deletions(-)

Comments

Jes Sorensen Oct. 27, 2014, 8:43 a.m. UTC | #1
Sanjeev Sharma <sanjeev_sharma@mentor.com> writes:
> This is a patch to the rtw_cmd.c file that fixes
> Error reported by checkpatch.
>
> Signed-off-by: Sanjeev Sharma <Sanjeev_Sharma@mentor.com>
> ---
>  drivers/staging/rtl8723au/core/rtw_cmd.c | 83 +++++++++++++++-----------------
>  1 file changed, 40 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/staging/rtl8723au/core/rtw_cmd.c b/drivers/staging/rtl8723au/core/rtw_cmd.c
> index 4eaa502..c1f6254 100644
> --- a/drivers/staging/rtl8723au/core/rtw_cmd.c
> +++ b/drivers/staging/rtl8723au/core/rtw_cmd.c
> @@ -957,7 +957,7 @@ static void traffic_status_watchdog(struct rtw_adapter *padapter)
>  		/*  check traffic for  powersaving. */
>  			if (((pmlmepriv->LinkDetectInfo.NumRxUnicastOkInPeriod +
>  			      pmlmepriv->LinkDetectInfo.NumTxOkInPeriod) > 8) ||
> -			    pmlmepriv->LinkDetectInfo.NumRxUnicastOkInPeriod >2)
> +			    pmlmepriv->LinkDetectInfo.NumRxUnicastOkInPeriod > 2)
>  				bEnterPS = false;
>  			else
>  				bEnterPS = true;

This makes the line longer than 80 characters, that is worse than the
'problem' you are fixing.

Jes
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sharma, Sanjeev Oct. 29, 2014, 6:32 a.m. UTC | #2
-----Original Message-----
From: Jes Sorensen [mailto:Jes.Sorensen@redhat.com] 
Sent: Monday, October 27, 2014 2:13 PM
To: Sharma, Sanjeev
Cc: Larry.Finger@lwfinger.net; gregkh@linuxfoundation.org; linux-wireless@vger.kernel.org; devel@driverdev.osuosl.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH] staging:rtl8723au: core: Fix error reported by checkpatch.

Sanjeev Sharma <sanjeev_sharma@mentor.com> writes:
> This is a patch to the rtw_cmd.c file that fixes Error reported by 
> checkpatch.
>
> Signed-off-by: Sanjeev Sharma <Sanjeev_Sharma@mentor.com>
> ---
>  drivers/staging/rtl8723au/core/rtw_cmd.c | 83 
> +++++++++++++++-----------------
>  1 file changed, 40 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/staging/rtl8723au/core/rtw_cmd.c 
> b/drivers/staging/rtl8723au/core/rtw_cmd.c
> index 4eaa502..c1f6254 100644
> --- a/drivers/staging/rtl8723au/core/rtw_cmd.c
> +++ b/drivers/staging/rtl8723au/core/rtw_cmd.c
> @@ -957,7 +957,7 @@ static void traffic_status_watchdog(struct rtw_adapter *padapter)
>  		/*  check traffic for  powersaving. */
>  			if (((pmlmepriv->LinkDetectInfo.NumRxUnicastOkInPeriod +
>  			      pmlmepriv->LinkDetectInfo.NumTxOkInPeriod) > 8) ||
> -			    pmlmepriv->LinkDetectInfo.NumRxUnicastOkInPeriod >2)
> +			    pmlmepriv->LinkDetectInfo.NumRxUnicastOkInPeriod > 2)
>  				bEnterPS = false;
>  			else
>  				bEnterPS = true;

This makes the line longer than 80 characters, that is worse than the 'problem' you are fixing.

Jes

Hello jes,

How it can be worse because checkpatch treating this as an Error and line longer than 80 character is warning reported by checkpatch and I could see that in entire staging directory,
every maintainer most of the time ignore the 80 column limit and give priority to Error.

Please let me know your comment .

Sanjeev Sharma 
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jes Sorensen Oct. 30, 2014, 2:49 p.m. UTC | #3
"Sharma, Sanjeev" <Sanjeev_Sharma@mentor.com> writes:
> -----Original Message-----
> From: Jes Sorensen [mailto:Jes.Sorensen@redhat.com] 
> Sent: Monday, October 27, 2014 2:13 PM
> To: Sharma, Sanjeev
> Cc: Larry.Finger@lwfinger.net; gregkh@linuxfoundation.org;
> linux-wireless@vger.kernel.org; devel@driverdev.osuosl.org;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] staging:rtl8723au: core: Fix error reported by checkpatch.
>
> Sanjeev Sharma <sanjeev_sharma@mentor.com> writes:
>> This is a patch to the rtw_cmd.c file that fixes Error reported by 
>> checkpatch.
>>
>> Signed-off-by: Sanjeev Sharma <Sanjeev_Sharma@mentor.com>
>> ---
>>  drivers/staging/rtl8723au/core/rtw_cmd.c | 83 
>> +++++++++++++++-----------------
>>  1 file changed, 40 insertions(+), 43 deletions(-)
>>
>> diff --git a/drivers/staging/rtl8723au/core/rtw_cmd.c 
>> b/drivers/staging/rtl8723au/core/rtw_cmd.c
>> index 4eaa502..c1f6254 100644
>> --- a/drivers/staging/rtl8723au/core/rtw_cmd.c
>> +++ b/drivers/staging/rtl8723au/core/rtw_cmd.c
>> @@ -957,7 +957,7 @@ static void traffic_status_watchdog(struct rtw_adapter *padapter)
>>  		/*  check traffic for  powersaving. */
>>  			if (((pmlmepriv->LinkDetectInfo.NumRxUnicastOkInPeriod +
>>  			      pmlmepriv->LinkDetectInfo.NumTxOkInPeriod) > 8) ||
>> -			    pmlmepriv->LinkDetectInfo.NumRxUnicastOkInPeriod >2)
>> +			    pmlmepriv->LinkDetectInfo.NumRxUnicastOkInPeriod > 2)
>>  				bEnterPS = false;
>>  			else
>>  				bEnterPS = true;
>
> This makes the line longer than 80 characters, that is worse than the 'problem' you are fixing.
>
> Jes
>
> Hello jes,
>
> How it can be worse because checkpatch treating this as an Error and
> line longer than 80 character is warning reported by checkpatch and I
> could see that in entire staging directory, 
> every maintainer most of the time ignore the 80 column limit and give
> priority to Error. 
>
> Please let me know your comment .
>
> Sanjeev Sharma 

checkpatch has it's ideas, doesn't mean it's blindly right at all
times. In this particular case it keeps the code more readable to keep
it below 80 characters than it does to add those two blanks and make the
line longer.

I agree the long variable names are nasty, and it doesn't help they were
done in StUdLyCaPs. If you want to attack it from that front, please go
ahead.

However, on formatting, please respond with proper email using proper
quoting when replying.

Thanks,
Jes
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/staging/rtl8723au/core/rtw_cmd.c b/drivers/staging/rtl8723au/core/rtw_cmd.c
index 4eaa502..c1f6254 100644
--- a/drivers/staging/rtl8723au/core/rtw_cmd.c
+++ b/drivers/staging/rtl8723au/core/rtw_cmd.c
@@ -957,7 +957,7 @@  static void traffic_status_watchdog(struct rtw_adapter *padapter)
 		/*  check traffic for  powersaving. */
 			if (((pmlmepriv->LinkDetectInfo.NumRxUnicastOkInPeriod +
 			      pmlmepriv->LinkDetectInfo.NumTxOkInPeriod) > 8) ||
-			    pmlmepriv->LinkDetectInfo.NumRxUnicastOkInPeriod >2)
+			    pmlmepriv->LinkDetectInfo.NumRxUnicastOkInPeriod > 2)
 				bEnterPS = false;
 			else
 				bEnterPS = true;
@@ -1017,46 +1017,44 @@  static void lps_ctrl_wk_hdl(struct rtw_adapter *padapter, u8 lps_ctrl_type)
 	    check_fwstate(pmlmepriv, WIFI_ADHOC_STATE))
 		return;
 
-	switch (lps_ctrl_type)
-	{
-		case LPS_CTRL_SCAN:
-			rtl8723a_BT_wifiscan_notify(padapter, true);
-			if (!rtl8723a_BT_using_antenna_1(padapter)) {
-				if (check_fwstate(pmlmepriv, _FW_LINKED))
-					LPS_Leave23a(padapter);
-			}
-			break;
-		case LPS_CTRL_JOINBSS:
+	switch (lps_ctrl_type) {
+	case LPS_CTRL_SCAN:
+		rtl8723a_BT_wifiscan_notify(padapter, true);
+		if (!rtl8723a_BT_using_antenna_1(padapter)) {
+			if (check_fwstate(pmlmepriv, _FW_LINKED))
 			LPS_Leave23a(padapter);
-			break;
-		case LPS_CTRL_CONNECT:
-			mstatus = 1;/* connect */
-			/*  Reset LPS Setting */
-			padapter->pwrctrlpriv.LpsIdleCount = 0;
-			rtl8723a_set_FwJoinBssReport_cmd(padapter, 1);
-			rtl8723a_BT_mediastatus_notify(padapter, mstatus);
-			break;
-		case LPS_CTRL_DISCONNECT:
-			mstatus = 0;/* disconnect */
-			rtl8723a_BT_mediastatus_notify(padapter, mstatus);
-			if (!rtl8723a_BT_using_antenna_1(padapter))
-				LPS_Leave23a(padapter);
-			rtl8723a_set_FwJoinBssReport_cmd(padapter, 0);
-			break;
-		case LPS_CTRL_SPECIAL_PACKET:
-			pwrpriv->DelayLPSLastTimeStamp = jiffies;
-			rtl8723a_BT_specialpacket_notify(padapter);
-			if (!rtl8723a_BT_using_antenna_1(padapter))
-				LPS_Leave23a(padapter);
-			break;
-		case LPS_CTRL_LEAVE:
-			rtl8723a_BT_lps_leave(padapter);
-			if (!rtl8723a_BT_using_antenna_1(padapter))
-				LPS_Leave23a(padapter);
-			break;
-
-		default:
-			break;
+		}
+		break;
+	case LPS_CTRL_JOINBSS:
+		LPS_Leave23a(padapter);
+		break;
+	case LPS_CTRL_CONNECT:
+		mstatus = 1;/* connect */
+		/*  Reset LPS Setting */
+		padapter->pwrctrlpriv.LpsIdleCount = 0;
+		rtl8723a_set_FwJoinBssReport_cmd(padapter, 1);
+		rtl8723a_BT_mediastatus_notify(padapter, mstatus);
+		break;
+	case LPS_CTRL_DISCONNECT:
+		mstatus = 0;/* disconnect */
+		rtl8723a_BT_mediastatus_notify(padapter, mstatus);
+		if (!rtl8723a_BT_using_antenna_1(padapter))
+			LPS_Leave23a(padapter);
+		rtl8723a_set_FwJoinBssReport_cmd(padapter, 0);
+		break;
+	case LPS_CTRL_SPECIAL_PACKET:
+		pwrpriv->DelayLPSLastTimeStamp = jiffies;
+		rtl8723a_BT_specialpacket_notify(padapter);
+		if (!rtl8723a_BT_using_antenna_1(padapter))
+			LPS_Leave23a(padapter);
+		break;
+	case LPS_CTRL_LEAVE:
+		rtl8723a_BT_lps_leave(padapter);
+		if (!rtl8723a_BT_using_antenna_1(padapter))
+			LPS_Leave23a(padapter);
+		break;
+	default:
+		break;
 	}
 }
 
@@ -1152,7 +1150,7 @@  static void rtw_chk_hi_queue_hdl(struct rtw_adapter *padapter)
 
 			cnt++;
 
-			if (cnt>10)
+			if (cnt > 10)
 				break;
 
 			val = rtl8723a_chk_hi_queue_empty(padapter);
@@ -1305,8 +1303,7 @@  int rtw_drvextra_cmd_hdl23a(struct rtw_adapter *padapter, const u8 *pbuf)
 
 	pdrvextra_cmd = (struct drvextra_cmd_parm *)pbuf;
 
-	switch (pdrvextra_cmd->ec_id)
-	{
+	switch (pdrvextra_cmd->ec_id) {
 	case DYNAMIC_CHK_WK_CID:
 		dynamic_chk_wk_hdl(padapter, pdrvextra_cmd->pbuf,
 				   pdrvextra_cmd->type_size);