[permalink] [raw]

Subject: Re: [PATCH] staging:rtl8723au: core: Fix Warning reported by checkpatch.

"Sharma, Sanjeev" <[email protected]> writes:
> -----Original Message-----
> From: Greg KH [mailto:[email protected]]
> Sent: Thursday, November 06, 2014 9:13 PM
> To: Sharma, Sanjeev
> Cc: [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]
> Subject: Re: [PATCH] staging:rtl8723au: core: Fix Warning reported by
> checkpatch.
>
> On Thu, Nov 06, 2014 at 12:06:36PM +0530, Sanjeev Sharma wrote:
>> This is a patch to the rtw_cmd.c file that fixes following Warning by
>> introducing temporary structure.
>>
>> WARNING: line over 80 characters
>>
>> Signed-off-by: Sanjeev Sharma <[email protected]>
>> ---
>> drivers/staging/rtl8723au/core/rtw_cmd.c | 123
>> +++++++++++++++----------------
>> 1 file changed, 60 insertions(+), 63 deletions(-)
>
> Same as the other patch, give us a hint as to the warning in the subject.
>
> This patch is Fix of Warning introduced in Previous patch while fixing " ERROR: spaces required around that '>' (ctx:WxV)".Can I mentioned dependency or hint in subject line or do we have
> another way to described these type of fix.(One patch introduced another Warning/Error)

When replying to these messages, please do it properly with proper
quoting. Do NOT resend other peoples' emails with just an
"-----Original Message-----" line and no comments and no indentation of
the message you respond to.

If you have any questions, please read:
https://www.ietf.org/rfc/rfc1855.txt

Jes

>
>>
>> diff --git a/drivers/staging/rtl8723au/core/rtw_cmd.c
>> b/drivers/staging/rtl8723au/core/rtw_cmd.c
>> index 4eaa502..6186575 100644
>> --- a/drivers/staging/rtl8723au/core/rtw_cmd.c
>> +++ b/drivers/staging/rtl8723au/core/rtw_cmd.c
>> @@ -919,34 +919,34 @@ static void traffic_status_watchdog(struct rtw_adapter *padapter)
>> u8 bHigherBusyTxTraffic = false;
>> struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
>> int BusyThreshold = 100;
>> + struct rt_link_detect *ldi = &pmlmepriv->LinkDetectInfo;
>> +
>> /* */
>> /* Determine if our traffic is busy now */
>> /* */
>> if (check_fwstate(pmlmepriv, _FW_LINKED)) {
>> if (rtl8723a_BT_coexist(padapter))
>> BusyThreshold = 50;
>> - else if (pmlmepriv->LinkDetectInfo.bBusyTraffic)
>> + else if (ldi->bBusyTraffic)
>> BusyThreshold = 75;
>> /* if we raise bBusyTraffic in last watchdog, using
>> lower threshold. */
>> - if (pmlmepriv->LinkDetectInfo.NumRxOkInPeriod > BusyThreshold ||
>> - pmlmepriv->LinkDetectInfo.NumTxOkInPeriod > BusyThreshold) {
>> + if (ldi->NumRxOkInPeriod > BusyThreshold ||
>> + ldi->NumTxOkInPeriod > BusyThreshold) {
>> bBusyTraffic = true;
>>
>> - if (pmlmepriv->LinkDetectInfo.NumRxOkInPeriod >
>> - pmlmepriv->LinkDetectInfo.NumTxOkInPeriod)
>> + if (ldi->NumRxOkInPeriod > ldi->NumTxOkInPeriod)
>> bRxBusyTraffic = true;
>> else
>> bTxBusyTraffic = true;
>> }
>>
>> /* Higher Tx/Rx data. */
>> - if (pmlmepriv->LinkDetectInfo.NumRxOkInPeriod > 4000 ||
>> - pmlmepriv->LinkDetectInfo.NumTxOkInPeriod > 4000) {
>> + if (ldi->NumRxOkInPeriod > 4000 ||
>> + ldi->NumTxOkInPeriod > 4000) {
>> bHigherBusyTraffic = true;
>>
>> - if (pmlmepriv->LinkDetectInfo.NumRxOkInPeriod >
>> - pmlmepriv->LinkDetectInfo.NumTxOkInPeriod)
>> + if (ldi->NumRxOkInPeriod > ldi->NumTxOkInPeriod)
>> bHigherBusyRxTraffic = true;
>> else
>> bHigherBusyTxTraffic = true;
>> @@ -955,9 +955,9 @@ static void traffic_status_watchdog(struct rtw_adapter *padapter)
>> if (!rtl8723a_BT_coexist(padapter) ||
>> !rtl8723a_BT_using_antenna_1(padapter)) {
>> /* check traffic for powersaving. */
>> - if (((pmlmepriv->LinkDetectInfo.NumRxUnicastOkInPeriod +
>> - pmlmepriv->LinkDetectInfo.NumTxOkInPeriod) > 8) ||
>> - pmlmepriv->LinkDetectInfo.NumRxUnicastOkInPeriod >2)
>> + if (((ldi->NumRxUnicastOkInPeriod +
>> + ldi->NumTxOkInPeriod) > 8) ||
>> + ldi->NumRxUnicastOkInPeriod > 2)
>> bEnterPS = false;
>> else
>> bEnterPS = true;
>> @@ -971,15 +971,15 @@ static void traffic_status_watchdog(struct rtw_adapter *padapter)
>> } else
>> LPS_Leave23a(padapter);
>>
>> - pmlmepriv->LinkDetectInfo.NumRxOkInPeriod = 0;
>> - pmlmepriv->LinkDetectInfo.NumTxOkInPeriod = 0;
>> - pmlmepriv->LinkDetectInfo.NumRxUnicastOkInPeriod = 0;
>> - pmlmepriv->LinkDetectInfo.bBusyTraffic = bBusyTraffic;
>> - pmlmepriv->LinkDetectInfo.bTxBusyTraffic = bTxBusyTraffic;
>> - pmlmepriv->LinkDetectInfo.bRxBusyTraffic = bRxBusyTraffic;
>> - pmlmepriv->LinkDetectInfo.bHigherBusyTraffic = bHigherBusyTraffic;
>> - pmlmepriv->LinkDetectInfo.bHigherBusyRxTraffic = bHigherBusyRxTraffic;
>> - pmlmepriv->LinkDetectInfo.bHigherBusyTxTraffic = bHigherBusyTxTraffic;
>> + ldi->NumRxOkInPeriod = 0;
>> + ldi->NumTxOkInPeriod = 0;
>> + ldi->NumRxUnicastOkInPeriod = 0;
>> + ldi->bBusyTraffic = bBusyTraffic;
>> + ldi->bTxBusyTraffic = bTxBusyTraffic;
>> + ldi->bRxBusyTraffic = bRxBusyTraffic;
>> + ldi->bHigherBusyTraffic = bHigherBusyTraffic;
>> + ldi->bHigherBusyRxTraffic = bHigherBusyRxTraffic;
>> + ldi->bHigherBusyTxTraffic = bHigherBusyTxTraffic;
>> }
>>
>> static void dynamic_chk_wk_hdl(struct rtw_adapter *padapter, u8
>> *pbuf, int sz) @@ -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:
>> - 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))
>> + 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);
>
> You are doing more than one thing here, something you did not specify in your changelog above, which isn't ok :(
>
> Please fix up and resend.
>
> thanks,
>
> greg k-h

2014-11-05 16:28:56

by Greg Kroah-Hartman

[permalink] [raw]

Subject: Re: [PATCH v2] staging:rtl8723au: core: Fix error reported by checkpatch.

On Wed, Nov 05, 2014 at 05:05:03PM +0530, Sanjeev Sharma wrote:
> This is a patch to the rtw_cmd.c file that fixes
> Error reported by checkpatch.

What error are you "fixing"? Please be specific.

thanks,

greg k-h

2014-11-05 11:35:16

by Sharma, Sanjeev

[permalink] [raw]

Subject: [PATCH v2] staging:rtl8723au: core: Fix error reported by checkpatch.

This is a patch to the rtw_cmd.c file that fixes
Error reported by checkpatch.

Signed-off-by: Sanjeev Sharma <[email protected]>
---
Changes in v2:
- Shorten variable name by taking temporary structure

drivers/staging/rtl8723au/core/rtw_cmd.c | 126 +++++++++++++++----------------
1 file changed, 62 insertions(+), 64 deletions(-)

diff --git a/drivers/staging/rtl8723au/core/rtw_cmd.c b/drivers/staging/rtl8723au/core/rtw_cmd.c
index 4eaa502..c6f6030 100644
--- a/drivers/staging/rtl8723au/core/rtw_cmd.c
+++ b/drivers/staging/rtl8723au/core/rtw_cmd.c
@@ -919,34 +919,34 @@ static void traffic_status_watchdog(struct rtw_adapter *padapter)
u8 bHigherBusyTxTraffic = false;
struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
int BusyThreshold = 100;
+ struct rt_link_detect *ldi = &pmlmepriv->LinkDetectInfo;
+
/* */
/* Determine if our traffic is busy now */
/* */
if (check_fwstate(pmlmepriv, _FW_LINKED)) {
if (rtl8723a_BT_coexist(padapter))
BusyThreshold = 50;
- else if (pmlmepriv->LinkDetectInfo.bBusyTraffic)
+ else if (ldi->bBusyTraffic)
BusyThreshold = 75;
/* if we raise bBusyTraffic in last watchdog, using
lower threshold. */
- if (pmlmepriv->LinkDetectInfo.NumRxOkInPeriod > BusyThreshold ||
- pmlmepriv->LinkDetectInfo.NumTxOkInPeriod > BusyThreshold) {
+ if (ldi->NumRxOkInPeriod > BusyThreshold ||
+ ldi->NumTxOkInPeriod > BusyThreshold) {
bBusyTraffic = true;

- if (pmlmepriv->LinkDetectInfo.NumRxOkInPeriod >
- pmlmepriv->LinkDetectInfo.NumTxOkInPeriod)
+ if (ldi->NumRxOkInPeriod > ldi->NumTxOkInPeriod)
bRxBusyTraffic = true;
else
bTxBusyTraffic = true;
}

/* Higher Tx/Rx data. */
- if (pmlmepriv->LinkDetectInfo.NumRxOkInPeriod > 4000 ||
- pmlmepriv->LinkDetectInfo.NumTxOkInPeriod > 4000) {
+ if (ldi->NumRxOkInPeriod > 4000 ||
+ ldi->NumTxOkInPeriod > 4000) {
bHigherBusyTraffic = true;

- if (pmlmepriv->LinkDetectInfo.NumRxOkInPeriod >
- pmlmepriv->LinkDetectInfo.NumTxOkInPeriod)
+ if (ldi->NumRxOkInPeriod > ldi->NumTxOkInPeriod)
bHigherBusyRxTraffic = true;
else
bHigherBusyTxTraffic = true;
@@ -955,9 +955,9 @@ static void traffic_status_watchdog(struct rtw_adapter *padapter)
if (!rtl8723a_BT_coexist(padapter) ||
!rtl8723a_BT_using_antenna_1(padapter)) {
/* check traffic for powersaving. */
- if (((pmlmepriv->LinkDetectInfo.NumRxUnicastOkInPeriod +
- pmlmepriv->LinkDetectInfo.NumTxOkInPeriod) > 8) ||
- pmlmepriv->LinkDetectInfo.NumRxUnicastOkInPeriod >2)
+ if (((ldi->NumRxUnicastOkInPeriod +
+ ldi->NumTxOkInPeriod) > 8) ||
+ ldi->NumRxUnicastOkInPeriod > 2)
bEnterPS = false;
else
bEnterPS = true;
@@ -968,18 +968,19 @@ static void traffic_status_watchdog(struct rtw_adapter *padapter)
else
LPS_Leave23a(padapter);
}
- } else
+ } else {
LPS_Leave23a(padapter);
+ }

- pmlmepriv->LinkDetectInfo.NumRxOkInPeriod = 0;
- pmlmepriv->LinkDetectInfo.NumTxOkInPeriod = 0;
- pmlmepriv->LinkDetectInfo.NumRxUnicastOkInPeriod = 0;
- pmlmepriv->LinkDetectInfo.bBusyTraffic = bBusyTraffic;
- pmlmepriv->LinkDetectInfo.bTxBusyTraffic = bTxBusyTraffic;
- pmlmepriv->LinkDetectInfo.bRxBusyTraffic = bRxBusyTraffic;
- pmlmepriv->LinkDetectInfo.bHigherBusyTraffic = bHigherBusyTraffic;
- pmlmepriv->LinkDetectInfo.bHigherBusyRxTraffic = bHigherBusyRxTraffic;
- pmlmepriv->LinkDetectInfo.bHigherBusyTxTraffic = bHigherBusyTxTraffic;
+ ldi->NumRxOkInPeriod = 0;
+ ldi->NumTxOkInPeriod = 0;
+ ldi->NumRxUnicastOkInPeriod = 0;
+ ldi->bBusyTraffic = bBusyTraffic;
+ ldi->bTxBusyTraffic = bTxBusyTraffic;
+ ldi->bRxBusyTraffic = bRxBusyTraffic;
+ ldi->bHigherBusyTraffic = bHigherBusyTraffic;
+ ldi->bHigherBusyRxTraffic = bHigherBusyRxTraffic;
+ ldi->bHigherBusyTxTraffic = bHigherBusyTxTraffic;
}

static void dynamic_chk_wk_hdl(struct rtw_adapter *padapter, u8 *pbuf, int sz)
@@ -1017,46 +1018,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 +1151,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 +1304,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);
--
1.7.11.7

2014-11-07 11:56:39

by Sharma, Sanjeev

[permalink] [raw]

Subject: RE: [PATCH] staging:rtl8723au: core: Fix error reported by checkpatch.

-----Original Message-----
From: Greg KH [mailto:[email protected]]
Sent: Thursday, November 06, 2014 9:11 PM
To: Sharma, Sanjeev
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH] staging:rtl8723au: core: Fix error reported by checkpatch.

Please specify the "error" in the subject in some way.

On Thu, Nov 06, 2014 at 11:46:13AM +0530, Sanjeev Sharma wrote:
> This is a patch to the rtw_cmd.c file that fixes following error.
>
> ERROR: spaces required around that '>' (ctx:WxV)
> ERROR: that open brace { should be on the previous line
>
> Signed-off-by: Sanjeev Sharma <[email protected]>
> ---
> drivers/staging/rtl8723au/core/rtw_cmd.c | 83
> +++++++++++++++-----------------
> 1 file changed, 40 insertions(+), 43 deletions(-)

This does two different things, please make it different patches.

For individual Error, is Separate patch needed. If Error has been mentioned in description. then it should be OK IMO.

And you sent 2 patches, with no hint as to which one comes before which.
Please resend all of them, correctly numbered in a series, so that I have a chance to get the order correct when applying them.

2014-11-06 15:42:31

by Greg Kroah-Hartman

[permalink] [raw]

Subject: Re: [PATCH] staging:rtl8723au: core: Fix error reported by checkpatch.

Please specify the "error" in the subject in some way.

On Thu, Nov 06, 2014 at 11:46:13AM +0530, Sanjeev Sharma wrote:
> This is a patch to the rtw_cmd.c file that fixes following
> error.
>
> ERROR: spaces required around that '>' (ctx:WxV)
> ERROR: that open brace { should be on the previous line
>
> Signed-off-by: Sanjeev Sharma <[email protected]>
> ---
> drivers/staging/rtl8723au/core/rtw_cmd.c | 83 +++++++++++++++-----------------
> 1 file changed, 40 insertions(+), 43 deletions(-)

This does two different things, please make it different patches.

And you sent 2 patches, with no hint as to which one comes before which.
Please resend all of them, correctly numbered in a series, so that I
have a chance to get the order correct when applying them.