-----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-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
