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

From: Sharma, Sanjeev
Date: Wed Nov 05 2014 - 06:38:14 EST


-----Original Message-----
From: Jes Sorensen [mailto:Jes.Sorensen@xxxxxxxxxx]
Sent: Thursday, October 30, 2014 8:21 PM
To: Sharma, Sanjeev
Cc: Joe Perches; Larry.Finger@xxxxxxxxxxxx; gregkh@xxxxxxxxxxxxxxxxxxx; linux-wireless@xxxxxxxxxxxxxxx; devel@xxxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
Subject: Re: [PATCH] staging:rtl8723au: core: Fix error reported by checkpatch.

"Sharma, Sanjeev" <Sanjeev_Sharma@xxxxxxxxxx> writes:
> -----Original Message-----
> From: Joe Perches [mailto:joe@xxxxxxxxxxx]
> Sent: Monday, October 27, 2014 8:23 PM
> To: Jes Sorensen
> Cc: Sharma, Sanjeev; Larry.Finger@xxxxxxxxxxxx;
> gregkh@xxxxxxxxxxxxxxxxxxx; linux-wireless@xxxxxxxxxxxxxxx;
> devel@xxxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> 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 <sanjeev_sharma@xxxxxxxxxx> 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)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/