2014-10-27 07:59:18

by Sharma, Sanjeev

Subject: [PATCH] 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]>
---
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;
@@ -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);
--
1.7.11.7

2014-10-27 08:44:44

by Jes Sorensen

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

Sanjeev Sharma <[email protected]> writes:
> This is a patch to the rtw_cmd.c file that fixes
> Error reported by checkpatch.
>
> Signed-off-by: Sanjeev Sharma <[email protected]>
> ---
> 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

2014-10-27 14:52:42

by Joe Perches

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

On Mon, 2014-10-27 at 09:43 +0100, Jes Sorensen wrote:
> Sanjeev Sharma <[email protected]> writes:
> > This is a patch to the rtw_cmd.c file that fixes
> > Error reported by checkpatch.
[]
> > diff --git 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.

The code already has dozens of long lines already.

This is generally a problem because the variable names
are pretty long so strict 80 column adherence generally
isn't possible.

A possible way to shorten these relatively long variable
name/line lengths is to use a temporary for

pmlmeprv->LinkDetectInfo

struct rt_link_detect *ldi = &pmlmeprv->LinkDetectInfo;

so:

drivers/staging/rtl8723au/core/rtw_cmd.c | 46 ++++++++++++++++----------------
1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/drivers/staging/rtl8723au/core/rtw_cmd.c b/drivers/staging/rtl8723au/core/rtw_cmd.c
index d2d7edf..1b24945 100644
--- a/drivers/staging/rtl8723au/core/rtw_cmd.c
+++ b/drivers/staging/rtl8723au/core/rtw_cmd.c
@@ -922,34 +922,33 @@ 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;
@@ -958,9 +957,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,18 +970,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)

2014-10-29 06:32:55

by Sharma, Sanjeev

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

-----Original Message-----
From: Jes Sorensen [mailto:[email protected]]
Sent: Monday, October 27, 2014 2:13 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.

Sanjeev Sharma <[email protected]> writes:
> This is a patch to the rtw_cmd.c file that fixes Error reported by
> checkpatch.
>
> Signed-off-by: Sanjeev Sharma <[email protected]>
> ---
> 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

2014-10-29 06:35:18

by Sharma, Sanjeev

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

-----Original Message-----
From: Joe Perches [mailto:[email protected]]
Sent: Monday, October 27, 2014 8:23 PM
To: Jes Sorensen
Cc: Sharma, Sanjeev; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH] staging:rtl8723au: core: Fix error reported by checkpatch.

On Mon, 2014-10-27 at 09:43 +0100, Jes Sorensen wrote:
> Sanjeev Sharma <[email protected]> writes:
> > This is a patch to the rtw_cmd.c file that fixes Error reported by
> > checkpatch.
[]
> > diff --git 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.

The code already has dozens of long lines already.

This is generally a problem because the variable names are pretty long so strict 80 column adherence generally isn't possible.

A possible way to shorten these relatively long variable name/line lengths is to use a temporary for

pmlmeprv->LinkDetectInfo

struct rt_link_detect *ldi = &pmlmeprv->LinkDetectInfo;

so:

I am agree on this approach but Let's wait for Jes opinion on it.

Sanjeev Sharma

drivers/staging/rtl8723au/core/rtw_cmd.c | 46 ++++++++++++++++----------------
1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/drivers/staging/rtl8723au/core/rtw_cmd.c b/drivers/staging/rtl8723au/core/rtw_cmd.c
index d2d7edf..1b24945 100644
--- a/drivers/staging/rtl8723au/core/rtw_cmd.c
+++ b/drivers/staging/rtl8723au/core/rtw_cmd.c
@@ -922,34 +922,33 @@ 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;
@@ -958,9 +957,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,18 +970,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)

2014-10-30 07:48:48

by Jes Sorensen

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

Joe Perches <[email protected]> writes:
> On Mon, 2014-10-27 at 09:43 +0100, Jes Sorensen wrote:
>> Sanjeev Sharma <[email protected]> writes:
>> > This is a patch to the rtw_cmd.c file that fixes
>> > Error reported by checkpatch.
> []
>> > diff --git 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.
>
> The code already has dozens of long lines already.
>
> This is generally a problem because the variable names
> are pretty long so strict 80 column adherence generally
> isn't possible.

It does, but that is not a reason to add more. I have generally tried to
trim it down when I cleaned up pieces of it.

Jes


> A possible way to shorten these relatively long variable
> name/line lengths is to use a temporary for
>
> pmlmeprv->LinkDetectInfo
>
> struct rt_link_detect *ldi = &pmlmeprv->LinkDetectInfo;
>
> so:
>
> drivers/staging/rtl8723au/core/rtw_cmd.c | 46 ++++++++++++++++----------------
> 1 file changed, 23 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/staging/rtl8723au/core/rtw_cmd.c b/drivers/staging/rtl8723au/core/rtw_cmd.c
> index d2d7edf..1b24945 100644
> --- a/drivers/staging/rtl8723au/core/rtw_cmd.c
> +++ b/drivers/staging/rtl8723au/core/rtw_cmd.c
> @@ -922,34 +922,33 @@ 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;
> @@ -958,9 +957,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,18 +970,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)

2014-10-30 14:49:29

by Jes Sorensen

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

"Sharma, Sanjeev" <[email protected]> writes:
> -----Original Message-----
> From: Jes Sorensen [mailto:[email protected]]
> Sent: Monday, October 27, 2014 2:13 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.
>
> Sanjeev Sharma <[email protected]> writes:
>> This is a patch to the rtw_cmd.c file that fixes Error reported by
>> checkpatch.
>>
>> Signed-off-by: Sanjeev Sharma <[email protected]>
>> ---
>> 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

2014-10-30 14:50:53

by Jes Sorensen

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

"Sharma, Sanjeev" <[email protected]> writes:
> -----Original Message-----
> From: Joe Perches [mailto:[email protected]]
> Sent: Monday, October 27, 2014 8:23 PM
> To: Jes Sorensen
> Cc: Sharma, Sanjeev; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: Re: [PATCH] staging:rtl8723au: core: Fix error reported by checkpatch.
>
> On Mon, 2014-10-27 at 09:43 +0100, Jes Sorensen wrote:
>> Sanjeev Sharma <[email protected]> writes:
>> > This is a patch to the rtw_cmd.c file that fixes Error reported by
>> > checkpatch.
> []
>> > diff --git 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.
>
> The code already has dozens of long lines already.
>
> This is generally a problem because the variable names are pretty long so strict 80 column adherence generally isn't possible.
>
> A possible way to shorten these relatively long variable name/line lengths is to use a temporary for
>
> pmlmeprv->LinkDetectInfo
>
> struct rt_link_detect *ldi = &pmlmeprv->LinkDetectInfo;
>
> so:
>
> I am agree on this approach but Let's wait for Jes opinion on it.
>
> Sanjeev Sharma
>
> drivers/staging/rtl8723au/core/rtw_cmd.c | 46 ++++++++++++++++----------------
> 1 file changed, 23 insertions(+), 23 deletions(-)

This is fine with me.

Jes

>
> diff --git a/drivers/staging/rtl8723au/core/rtw_cmd.c b/drivers/staging/rtl8723au/core/rtw_cmd.c
> index d2d7edf..1b24945 100644
> --- a/drivers/staging/rtl8723au/core/rtw_cmd.c
> +++ b/drivers/staging/rtl8723au/core/rtw_cmd.c
> @@ -922,34 +922,33 @@ 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;
> @@ -958,9 +957,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,18 +970,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)

2014-11-05 11:35:18

by Sharma, Sanjeev

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-05 11:38:11

by Sharma, Sanjeev

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

-----Original Message-----
From: Jes Sorensen [mailto:[email protected]]
Sent: Thursday, October 30, 2014 8:21 PM
To: Sharma, Sanjeev
Cc: Joe Perches; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH] staging:rtl8723au: core: Fix error reported by checkpatch.

"Sharma, Sanjeev" <[email protected]> writes:
> -----Original Message-----
> From: Joe Perches [mailto:[email protected]]
> Sent: Monday, October 27, 2014 8:23 PM
> To: Jes Sorensen
> Cc: Sharma, Sanjeev; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: Re: [PATCH] staging:rtl8723au: core: Fix error reported by checkpatch.
>
> On Mon, 2014-10-27 at 09:43 +0100, Jes Sorensen wrote:
>> Sanjeev Sharma <[email protected]> writes:
>> > This is a patch to the rtw_cmd.c file that fixes Error reported by
>> > checkpatch.
> []
>> > diff --git 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.
>
> The code already has dozens of long lines already.
>
> This is generally a problem because the variable names are pretty long so strict 80 column adherence generally isn't possible.
>
> A possible way to shorten these relatively long variable name/line
> lengths is to use a temporary for
>
> pmlmeprv->LinkDetectInfo
>
> struct rt_link_detect *ldi = &pmlmeprv->LinkDetectInfo;
>
> so:
>
> I am agree on this approach but Let's wait for Jes opinion on it.
>
> Sanjeev Sharma
>
> drivers/staging/rtl8723au/core/rtw_cmd.c | 46
> ++++++++++++++++----------------
> 1 file changed, 23 insertions(+), 23 deletions(-)

This is fine with me.

Jes

Summited another patch after incorporating the change.

Sanjeev Sharma
>
> diff --git a/drivers/staging/rtl8723au/core/rtw_cmd.c
> b/drivers/staging/rtl8723au/core/rtw_cmd.c
> index d2d7edf..1b24945 100644
> --- a/drivers/staging/rtl8723au/core/rtw_cmd.c
> +++ b/drivers/staging/rtl8723au/core/rtw_cmd.c
> @@ -922,34 +922,33 @@ 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;
> @@ -958,9 +957,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,18 +970,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)

2014-11-05 16:28:58

by Greg Kroah-Hartman

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 17:16:00

by Joe Perches

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

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

Please run your patches through checkpatch before
sending them.

WARNING: suspect code indent for conditional statements (24, 24)
#178: FILE: drivers/staging/rtl8723au/core/rtw_cmd.c:1025:
+ if (check_fwstate(pmlmepriv, _FW_LINKED))
LPS_Leave23a(padapter);

Also, there are a couple of different things
you changing here.

This should be 2 separate patches.

One to use the temporary for:
+ struct rt_link_detect *ldi = &pmlmepriv->LinkDetectInfo;

(and that bit has a defect here:


+ } else {
LPS_Leave23a(padapter);
+ }

where the indentation for the last close brace is wrong)

and another for the whitespace only changes

2014-11-06 05:41:18

by Sharma, Sanjeev

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

-----Original Message-----
From: Joe Perches [mailto:[email protected]]
Sent: Wednesday, November 05, 2014 10:46 PM
To: Sharma, Sanjeev
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH v2] staging:rtl8723au: core: Fix error reported by checkpatch.

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

Please run your patches through checkpatch before sending them.

In this patch I am fixing error reported by check patch since Error has higher priority.

WARNING: suspect code indent for conditional statements (24, 24)
#178: FILE: drivers/staging/rtl8723au/core/rtw_cmd.c:1025:
+ if (check_fwstate(pmlmepriv, _FW_LINKED))
LPS_Leave23a(padapter);

Also, there are a couple of different things you changing here.

This should be 2 separate patches.

Ok I will come up with 2 patch separately.

One to use the temporary for:
+ struct rt_link_detect *ldi = &pmlmepriv->LinkDetectInfo;

(and that bit has a defect here:


+ } else {
LPS_Leave23a(padapter);
+ }

where the indentation for the last close brace is wrong)

and another for the whitespace only changes

2014-11-06 06:16:26

by Sharma, Sanjeev

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

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(-)

diff --git a/drivers/staging/rtl8723au/core/rtw_cmd.c b/drivers/staging/rtl8723au/core/rtw_cmd.c
index 4eaa502..e9d6ac2 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:
- 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))
+ 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_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);
--
1.7.11.7

2014-11-06 06:36:46

by Sharma, Sanjeev

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

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(-)

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);
- 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);
--
1.7.11.7

2014-11-06 06:37:34

by Sharma, Sanjeev

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

-----Original Message-----
From: Greg KH [mailto:[email protected]]
Sent: Wednesday, November 05, 2014 9:58 PM
To: Sharma, Sanjeev
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
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.

Submitted individual patch with detail of fix.

thanks,

greg k-h

2014-11-06 15:42:35

by Greg Kroah-Hartman

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.

2014-11-06 15:44:17

by Greg Kroah-Hartman

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.


>
> 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-07 11:56:42

by Sharma, Sanjeev

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-07 12:00:25

by Sharma, Sanjeev

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

-----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)

>
> 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-07 15:46:44

by Jes Sorensen

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-11 07:54:55

by Sharma, Sanjeev

Subject: [PATCH] staging:rtl8723au: core: Added missing space reported by checkpatch.

This is a patch to the rtw_cmd.c file that fixes following
Error.

ERROR: spaces required around that '>' (ctx:WxV)

Signed-off-by: Sanjeev Sharma <[email protected]>
---
drivers/staging/rtl8723au/core/rtw_cmd.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8723au/core/rtw_cmd.c b/drivers/staging/rtl8723au/core/rtw_cmd.c
index 4eaa502..cd2e915 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;
@@ -1152,7 +1152,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);
--
1.7.11.7

2014-11-11 09:58:26

by Sharma, Sanjeev

Subject: [PATCH] staging:rtl8723au: core: Fix checkpatch warning

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 | 42 ++++++++++++++++----------------
1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/drivers/staging/rtl8723au/core/rtw_cmd.c b/drivers/staging/rtl8723au/core/rtw_cmd.c
index cd2e915..6eccd86 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)
--
1.7.11.7