2015-02-10 13:01:39

by Pushpendra Singh

[permalink] [raw]
Subject: [PATCH 1/6] staging: rt8188eu: hal: removed space before ','

Removed checkpatch.pl error
ERROR: space prohibited before that ',' (ctx:WxW)

Signed-off-by: Pushpendra Singh <[email protected]>
---
drivers/staging/rtl8188eu/hal/Hal8188ERateAdaptive.c | 4 ++--
drivers/staging/rtl8188eu/hal/odm.c | 8 ++++----
drivers/staging/rtl8188eu/hal/pwrseqcmd.c | 2 +-
drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c | 6 +++---
drivers/staging/rtl8188eu/hal/usb_halinit.c | 2 +-
5 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/rtl8188eu/hal/Hal8188ERateAdaptive.c b/drivers/staging/rtl8188eu/hal/Hal8188ERateAdaptive.c
index 3c651d5..b5e5c93 100644
--- a/drivers/staging/rtl8188eu/hal/Hal8188ERateAdaptive.c
+++ b/drivers/staging/rtl8188eu/hal/Hal8188ERateAdaptive.c
@@ -87,7 +87,7 @@ static u8 DROPING_NECESSARY[RATESIZE] = {

static u8 PendingForRateUpFail[5] = {2, 10, 24, 40, 60};
static u16 DynamicTxRPTTiming[6] = {
- 0x186a, 0x30d4, 0x493e, 0x61a8, 0x7a12 , 0x927c}; /* 200ms-1200ms */
+ 0x186a, 0x30d4, 0x493e, 0x61a8, 0x7a12, 0x927c}; /* 200ms-1200ms */

/* End Rate adaptive parameters */

@@ -714,7 +714,7 @@ void ODM_RA_TxRPT2Handle_8188E(struct odm_dm_struct *dm_odm, u8 *TxRPT_Buf, u16
pRAInfo->RTY[0], pRAInfo->RTY[1],
pRAInfo->RTY[2], pRAInfo->RTY[3],
pRAInfo->RTY[4], pRAInfo->DROP,
- macid_entry0 , macid_entry1));
+ macid_entry0, macid_entry1));
if (pRAInfo->PTActive) {
if (pRAInfo->RAstage < 5)
odm_RateDecision_8188E(dm_odm, pRAInfo);
diff --git a/drivers/staging/rtl8188eu/hal/odm.c b/drivers/staging/rtl8188eu/hal/odm.c
index 06477e8..d12eac4 100644
--- a/drivers/staging/rtl8188eu/hal/odm.c
+++ b/drivers/staging/rtl8188eu/hal/odm.c
@@ -881,7 +881,7 @@ void ODM_RF_Saving(struct odm_dm_struct *pDM_Odm, u8 bForceInNormal)

if (pDM_PSTable->PreRFState != pDM_PSTable->CurRFState) {
if (pDM_PSTable->CurRFState == RF_Save) {
- phy_set_bb_reg(adapter, 0x874 , 0x1C0000, 0x2); /* Reg874[20:18]=3'b010 */
+ phy_set_bb_reg(adapter, 0x874, 0x1C0000, 0x2); /* Reg874[20:18]=3'b010 */
phy_set_bb_reg(adapter, 0xc70, BIT3, 0); /* RegC70[3]=1'b0 */
phy_set_bb_reg(adapter, 0x85c, 0xFF000000, 0x63); /* Reg85C[31:24]=0x63 */
phy_set_bb_reg(adapter, 0x874, 0xC000, 0x2); /* Reg874[15:14]=2'b10 */
@@ -889,7 +889,7 @@ void ODM_RF_Saving(struct odm_dm_struct *pDM_Odm, u8 bForceInNormal)
phy_set_bb_reg(adapter, 0x818, BIT28, 0x0); /* Reg818[28]=1'b0 */
phy_set_bb_reg(adapter, 0x818, BIT28, 0x1); /* Reg818[28]=1'b1 */
} else {
- phy_set_bb_reg(adapter, 0x874 , 0x1CC000, pDM_PSTable->Reg874);
+ phy_set_bb_reg(adapter, 0x874, 0x1CC000, pDM_PSTable->Reg874);
phy_set_bb_reg(adapter, 0xc70, BIT3, pDM_PSTable->RegC70);
phy_set_bb_reg(adapter, 0x85c, 0xFF000000, pDM_PSTable->Reg85C);
phy_set_bb_reg(adapter, 0xa74, 0xF000, pDM_PSTable->RegA74);
@@ -1043,7 +1043,7 @@ void odm_RefreshRateAdaptiveMaskCE(struct odm_dm_struct *pDM_Odm)
for (i = 0; i < ODM_ASSOCIATE_ENTRY_NUM; i++) {
struct sta_info *pstat = pDM_Odm->pODM_StaInfo[i];
if (IS_STA_VALID(pstat)) {
- if (ODM_RAStateCheck(pDM_Odm, pstat->rssi_stat.UndecoratedSmoothedPWDB, false , &pstat->rssi_level)) {
+ if (ODM_RAStateCheck(pDM_Odm, pstat->rssi_stat.UndecoratedSmoothedPWDB, false, &pstat->rssi_level)) {
ODM_RT_TRACE(pDM_Odm, ODM_COMP_RA_MASK, ODM_DBG_LOUD,
("RSSI:%d, RSSI_LEVEL:%d\n",
pstat->rssi_stat.UndecoratedSmoothedPWDB, pstat->rssi_level));
@@ -1191,7 +1191,7 @@ void odm_RSSIMonitorCheckCE(struct odm_dm_struct *pDM_Odm)
pdmpriv->EntryMinUndecoratedSmoothedPWDB = 0;

FindMinimumRSSI(Adapter);
- ODM_CmnInfoUpdate(&pHalData->odmpriv , ODM_CMNINFO_RSSI_MIN, pdmpriv->MinUndecoratedPWDBForDM);
+ ODM_CmnInfoUpdate(&pHalData->odmpriv, ODM_CMNINFO_RSSI_MIN, pdmpriv->MinUndecoratedPWDBForDM);
}

/* 3============================================================ */
diff --git a/drivers/staging/rtl8188eu/hal/pwrseqcmd.c b/drivers/staging/rtl8188eu/hal/pwrseqcmd.c
index be0663e..768bf57 100644
--- a/drivers/staging/rtl8188eu/hal/pwrseqcmd.c
+++ b/drivers/staging/rtl8188eu/hal/pwrseqcmd.c
@@ -37,7 +37,7 @@ u8 rtl88eu_pwrseqcmdparsing(struct adapter *padapter, u8 cut_vers, u8 fab_vers,
do {
pwrcfgcmd = pwrseqcmd[aryidx];

- RT_TRACE(_module_hal_init_c_ , _drv_info_,
+ RT_TRACE(_module_hal_init_c_, _drv_info_,
("rtl88eu_pwrseqcmdparsing: offset(%#x) cut_msk(%#x)"
"fab_msk(%#x) interface_msk(%#x) base(%#x) cmd(%#x)"
"msk(%#x) value(%#x)\n",
diff --git a/drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c b/drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c
index 3222d8d..1aae302 100644
--- a/drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c
+++ b/drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c
@@ -519,7 +519,7 @@ void Hal_ReadPowerSavingMode88E(struct adapter *padapter, u8 *hwinfo, bool AutoL
padapter->pwrctrlpriv.bSupportRemoteWakeup = (hwinfo[EEPROM_USB_OPTIONAL_FUNCTION0] & BIT1) ? true : false;

DBG_88E("%s...bHWPwrPindetect(%x)-bHWPowerdown(%x) , bSupportRemoteWakeup(%x)\n", __func__,
- padapter->pwrctrlpriv.bHWPwrPindetect, padapter->pwrctrlpriv.bHWPowerdown , padapter->pwrctrlpriv.bSupportRemoteWakeup);
+ padapter->pwrctrlpriv.bHWPwrPindetect, padapter->pwrctrlpriv.bHWPowerdown, padapter->pwrctrlpriv.bSupportRemoteWakeup);

DBG_88E("### PS params => power_mgnt(%x), usbss_enable(%x) ###\n", padapter->registrypriv.power_mgnt, padapter->registrypriv.usbss_enable);
}
@@ -549,8 +549,8 @@ void Hal_ReadTxPowerInfo88E(struct adapter *padapter, u8 *PROMContent, bool Auto
}
if (bIn24G) {
DBG_88E("======= Path %d, Channel %d =======\n", rfPath, ch);
- DBG_88E("Index24G_CCK_Base[%d][%d] = 0x%x\n", rfPath, ch , pHalData->Index24G_CCK_Base[rfPath][ch]);
- DBG_88E("Index24G_BW40_Base[%d][%d] = 0x%x\n", rfPath, ch , pHalData->Index24G_BW40_Base[rfPath][ch]);
+ DBG_88E("Index24G_CCK_Base[%d][%d] = 0x%x\n", rfPath, ch, pHalData->Index24G_CCK_Base[rfPath][ch]);
+ DBG_88E("Index24G_BW40_Base[%d][%d] = 0x%x\n", rfPath, ch, pHalData->Index24G_BW40_Base[rfPath][ch]);
}
}
for (TxCount = 0; TxCount < MAX_TX_COUNT; TxCount++) {
diff --git a/drivers/staging/rtl8188eu/hal/usb_halinit.c b/drivers/staging/rtl8188eu/hal/usb_halinit.c
index 14650e9..0078468 100644
--- a/drivers/staging/rtl8188eu/hal/usb_halinit.c
+++ b/drivers/staging/rtl8188eu/hal/usb_halinit.c
@@ -1796,7 +1796,7 @@ static void SetHwReg8188EU(struct adapter *Adapter, u8 variable, u8 *val)
}
break;
case HW_VAR_H2C_MEDIA_STATUS_RPT:
- rtl8188e_set_FwMediaStatus_cmd(Adapter , (*(__le16 *)val));
+ rtl8188e_set_FwMediaStatus_cmd(Adapter, (*(__le16 *)val));
break;
case HW_VAR_BCN_VALID:
/* BCN_VALID, BIT16 of REG_TDECTRL = BIT0 of REG_TDECTRL+2, write 1 to clear, Clear by sw */
--
1.9.1


2015-02-10 13:01:45

by Pushpendra Singh

[permalink] [raw]
Subject: [PATCH 2/6] staging: rtl8188eu: hal: removed code indent error

Removed checkpatch.pl error
ERROR: code indent should use tabs where possible

Signed-off-by: Pushpendra Singh <[email protected]>
---
drivers/staging/rtl8188eu/hal/rf.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8188eu/hal/rf.c b/drivers/staging/rtl8188eu/hal/rf.c
index eea4c8a..88a026c 100644
--- a/drivers/staging/rtl8188eu/hal/rf.c
+++ b/drivers/staging/rtl8188eu/hal/rf.c
@@ -101,8 +101,7 @@ void rtl88eu_phy_rf6052_set_cck_txpower(struct adapter *adapt, u8 *powerlevel)
ptr++;
}
}
- rtl88eu_dm_txpower_track_adjust(&hal_data->odmpriv, 1, &direction,
- &pwrtrac_value);
+ rtl88eu_dm_txpower_track_adjust(&hal_data->odmpriv, 1, &direction, &pwrtrac_value);

if (direction == 1) {
/* Increase TX power */
--
1.9.1

2015-02-10 13:01:49

by Pushpendra Singh

[permalink] [raw]
Subject: [PATCH 3/6] staging: rtl8188eu: hal: removed unnecessary braces

Removed checkpatch.pl below warning
WARNING: braces {} are not necessary for single statement blocks

Signed-off-by: Pushpendra Singh <[email protected]>
---
drivers/staging/rtl8188eu/hal/bb_cfg.c | 15 +++++++--------
drivers/staging/rtl8188eu/hal/fw.c | 3 +--
drivers/staging/rtl8188eu/hal/odm_HWConfig.c | 5 ++---
drivers/staging/rtl8188eu/hal/odm_RTL8188E.c | 3 +--
drivers/staging/rtl8188eu/hal/phy.c | 12 ++++--------
drivers/staging/rtl8188eu/hal/rtl8188e_cmd.c | 6 ++----
drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c | 3 +--
drivers/staging/rtl8188eu/hal/usb_halinit.c | 3 +--
8 files changed, 19 insertions(+), 31 deletions(-)

diff --git a/drivers/staging/rtl8188eu/hal/bb_cfg.c b/drivers/staging/rtl8188eu/hal/bb_cfg.c
index 1e963bf..76d3db1 100644
--- a/drivers/staging/rtl8188eu/hal/bb_cfg.c
+++ b/drivers/staging/rtl8188eu/hal/bb_cfg.c
@@ -554,21 +554,20 @@ static void store_pwrindex_offset(struct adapter *Adapter, u32 regaddr, u32 bitm

static void rtl_addr_delay(struct adapter *adapt, u32 addr, u32 bit_mask, u32 data)
{
- if (addr == 0xfe) {
+ if (addr == 0xfe)
msleep(50);
- } else if (addr == 0xfd) {
+ else if (addr == 0xfd)
mdelay(5);
- } else if (addr == 0xfc) {
+ else if (addr == 0xfc)
mdelay(1);
- } else if (addr == 0xfb) {
+ else if (addr == 0xfb)
udelay(50);
- } else if (addr == 0xfa) {
+ else if (addr == 0xfa)
udelay(5);
- } else if (addr == 0xf9) {
+ else if (addr == 0xf9)
udelay(1);
- } else{
+ else
store_pwrindex_offset(adapt, addr, bit_mask, data);
- }
}

static bool config_bb_with_pgheader(struct adapter *adapt)
diff --git a/drivers/staging/rtl8188eu/hal/fw.c b/drivers/staging/rtl8188eu/hal/fw.c
index 3b28754..a71c542 100644
--- a/drivers/staging/rtl8188eu/hal/fw.c
+++ b/drivers/staging/rtl8188eu/hal/fw.c
@@ -154,9 +154,8 @@ static int _rtl88e_fw_free_to_go(struct adapter *adapt)
break;
} while (counter++ < POLLING_READY_TIMEOUT_COUNT);

- if (counter >= POLLING_READY_TIMEOUT_COUNT) {
+ if (counter >= POLLING_READY_TIMEOUT_COUNT)
goto exit;
- }

value32 = usb_read32(adapt, REG_MCUFWDL);
value32 |= MCUFWDL_RDY;
diff --git a/drivers/staging/rtl8188eu/hal/odm_HWConfig.c b/drivers/staging/rtl8188eu/hal/odm_HWConfig.c
index 29f87df..7213187 100644
--- a/drivers/staging/rtl8188eu/hal/odm_HWConfig.c
+++ b/drivers/staging/rtl8188eu/hal/odm_HWConfig.c
@@ -418,11 +418,10 @@ static void ODM_PhyStatusQuery_92CSeries(struct odm_dm_struct *dm_odm,
{
odm_RxPhyStatus92CSeries_Parsing(dm_odm, pPhyInfo, pPhyStatus,
pPktinfo);
- if (dm_odm->RSSI_test) {
+ if (dm_odm->RSSI_test)
;/* Select the packets to do RSSI checking for antenna switching. */
- } else {
+ else
odm_Process_RSSIForDM(dm_odm, pPhyInfo, pPktinfo);
- }
}

void ODM_PhyStatusQuery(struct odm_dm_struct *dm_odm,
diff --git a/drivers/staging/rtl8188eu/hal/odm_RTL8188E.c b/drivers/staging/rtl8188eu/hal/odm_RTL8188E.c
index d3c6873..fd7930f 100644
--- a/drivers/staging/rtl8188eu/hal/odm_RTL8188E.c
+++ b/drivers/staging/rtl8188eu/hal/odm_RTL8188E.c
@@ -103,9 +103,8 @@ static void dm_fast_training_init(struct odm_dm_struct *dm_odm)
struct fast_ant_train *dm_fat_tbl = &dm_odm->DM_FatTable;
u32 AntCombination = 2;

- if (*(dm_odm->mp_mode) == 1) {
+ if (*(dm_odm->mp_mode) == 1)
return;
- }

for (i = 0; i < 6; i++) {
dm_fat_tbl->Bssid[i] = 0;
diff --git a/drivers/staging/rtl8188eu/hal/phy.c b/drivers/staging/rtl8188eu/hal/phy.c
index 3f663fe..0a1a7d6 100644
--- a/drivers/staging/rtl8188eu/hal/phy.c
+++ b/drivers/staging/rtl8188eu/hal/phy.c
@@ -991,9 +991,8 @@ static void save_adda_registers(struct adapter *adapt, u32 *addareg,
{
u32 i;

- for (i = 0; i < register_num; i++) {
+ for (i = 0; i < register_num; i++)
backup[i] = phy_query_bb_reg(adapt, addareg[i], bMaskDWord);
- }
}

static void save_mac_registers(struct adapter *adapt, u32 *mac_reg,
@@ -1001,9 +1000,8 @@ static void save_mac_registers(struct adapter *adapt, u32 *mac_reg,
{
u32 i;

- for (i = 0; i < (IQK_MAC_REG_NUM - 1); i++) {
+ for (i = 0; i < (IQK_MAC_REG_NUM - 1); i++)
backup[i] = usb_read8(adapt, mac_reg[i]);
- }
backup[i] = usb_read32(adapt, mac_reg[i]);
}

@@ -1021,9 +1019,8 @@ static void reload_mac_registers(struct adapter *adapt,
{
u32 i;

- for (i = 0; i < (IQK_MAC_REG_NUM - 1); i++) {
+ for (i = 0; i < (IQK_MAC_REG_NUM - 1); i++)
usb_write8(adapt, mac_reg[i], (u8)backup[i]);
- }
usb_write32(adapt, mac_reg[i], backup[i]);
}

@@ -1051,9 +1048,8 @@ static void mac_setting_calibration(struct adapter *adapt, u32 *mac_reg, u32 *ba

usb_write8(adapt, mac_reg[i], 0x3F);

- for (i = 1; i < (IQK_MAC_REG_NUM - 1); i++) {
+ for (i = 1; i < (IQK_MAC_REG_NUM - 1); i++)
usb_write8(adapt, mac_reg[i], (u8)(backup[i]&(~BIT3)));
- }
usb_write8(adapt, mac_reg[i], (u8)(backup[i]&(~BIT5)));
}

diff --git a/drivers/staging/rtl8188eu/hal/rtl8188e_cmd.c b/drivers/staging/rtl8188eu/hal/rtl8188e_cmd.c
index 86347f2..cf2609f 100644
--- a/drivers/staging/rtl8188eu/hal/rtl8188e_cmd.c
+++ b/drivers/staging/rtl8188eu/hal/rtl8188e_cmd.c
@@ -104,15 +104,13 @@ static s32 FillH2CCmd_88E(struct adapter *adapt, u8 ElementID, u32 CmdLen, u8 *p

/* Write Ext command */
msgbox_ex_addr = REG_HMEBOX_EXT_0 + (h2c_box_num * RTL88E_EX_MESSAGE_BOX_SIZE);
- for (cmd_idx = 0; cmd_idx < ext_cmd_len; cmd_idx++) {
+ for (cmd_idx = 0; cmd_idx < ext_cmd_len; cmd_idx++)
usb_write8(adapt, msgbox_ex_addr+cmd_idx, *((u8 *)(&h2c_cmd_ex)+cmd_idx));
- }
}
/* Write command */
msgbox_addr = REG_HMEBOX_0 + (h2c_box_num * RTL88E_MESSAGE_BOX_SIZE);
- for (cmd_idx = 0; cmd_idx < RTL88E_MESSAGE_BOX_SIZE; cmd_idx++) {
+ for (cmd_idx = 0; cmd_idx < RTL88E_MESSAGE_BOX_SIZE; cmd_idx++)
usb_write8(adapt, msgbox_addr+cmd_idx, *((u8 *)(&h2c_cmd)+cmd_idx));
- }
bcmd_down = true;

haldata->LastHMEBoxNum = (h2c_box_num+1) % RTL88E_MAX_H2C_BOX_NUMS;
diff --git a/drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c b/drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c
index 1aae302..c727a78 100644
--- a/drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c
+++ b/drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c
@@ -303,9 +303,8 @@ s32 InitLLTTable(struct adapter *padapter, u8 txpktbuf_bndy)

/* Let last entry point to the start entry of ring buffer */
status = _LLTWrite(padapter, Last_Entry_Of_TxPktBuf, txpktbuf_bndy);
- if (_SUCCESS != status) {
+ if (_SUCCESS != status)
return status;
- }
}

return status;
diff --git a/drivers/staging/rtl8188eu/hal/usb_halinit.c b/drivers/staging/rtl8188eu/hal/usb_halinit.c
index 0078468..5665a7d 100644
--- a/drivers/staging/rtl8188eu/hal/usb_halinit.c
+++ b/drivers/staging/rtl8188eu/hal/usb_halinit.c
@@ -716,9 +716,8 @@ static u32 rtl8188eu_hal_init(struct adapter *Adapter)
/* Save target channel */
haldata->CurrentChannel = 6;/* default set to 6 */

- if (pwrctrlpriv->reg_rfoff) {
+ if (pwrctrlpriv->reg_rfoff)
pwrctrlpriv->rf_pwrstate = rf_off;
- }

/* 2010/08/09 MH We need to check if we need to turnon or off RF after detecting */
/* HW GPIO pin. Before PHY_RFConfig8192C. */
--
1.9.1

2015-02-10 13:01:55

by Pushpendra Singh

[permalink] [raw]
Subject: [PATCH 4/6] staging: rtl8188eu: hal: added blank line after declarations

Removed checkpatch.pl below warning
WARNING: Missing a blank line after declarations

Signed-off-by: Pushpendra Singh <[email protected]>
---
.../staging/rtl8188eu/hal/Hal8188ERateAdaptive.c | 1 +
drivers/staging/rtl8188eu/hal/hal_intf.c | 2 ++
drivers/staging/rtl8188eu/hal/odm.c | 3 +++
drivers/staging/rtl8188eu/hal/odm_RTL8188E.c | 1 +
drivers/staging/rtl8188eu/hal/phy.c | 1 +
drivers/staging/rtl8188eu/hal/rtl8188e_cmd.c | 1 +
drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c | 2 ++
drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c | 3 ++-
drivers/staging/rtl8188eu/hal/usb_halinit.c | 22 +++++++++++++++++++---
9 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/rtl8188eu/hal/Hal8188ERateAdaptive.c b/drivers/staging/rtl8188eu/hal/Hal8188ERateAdaptive.c
index b5e5c93..4a0504d 100644
--- a/drivers/staging/rtl8188eu/hal/Hal8188ERateAdaptive.c
+++ b/drivers/staging/rtl8188eu/hal/Hal8188ERateAdaptive.c
@@ -538,6 +538,7 @@ int ODM_RAInfo_Init(struct odm_dm_struct *dm_odm, u8 macid)
struct odm_ra_info *pRaInfo = &dm_odm->RAInfo[macid];
u8 WirelessMode = 0xFF; /* invalid value */
u8 max_rate_idx = 0x13; /* MCS7 */
+
if (dm_odm->pWirelessMode != NULL)
WirelessMode = *(dm_odm->pWirelessMode);

diff --git a/drivers/staging/rtl8188eu/hal/hal_intf.c b/drivers/staging/rtl8188eu/hal/hal_intf.c
index 4bdbed2..5edb5c4 100644
--- a/drivers/staging/rtl8188eu/hal/hal_intf.c
+++ b/drivers/staging/rtl8188eu/hal/hal_intf.c
@@ -202,6 +202,7 @@ s32 rtw_hal_xmit(struct adapter *adapt, struct xmit_frame *pxmitframe)
s32 rtw_hal_mgnt_xmit(struct adapter *adapt, struct xmit_frame *pmgntframe)
{
s32 ret = _FAIL;
+
if (adapt->HalFunc.mgnt_xmit)
ret = adapt->HalFunc.mgnt_xmit(adapt, pmgntframe);
return ret;
@@ -236,6 +237,7 @@ void rtw_hal_update_ra_mask(struct adapter *adapt, u32 mac_id, u8 rssi_level)
#ifdef CONFIG_88EU_AP_MODE
struct sta_info *psta = NULL;
struct sta_priv *pstapriv = &adapt->stapriv;
+
if ((mac_id-1) > 0)
psta = pstapriv->sta_aid[(mac_id-1) - 1];
if (psta)
diff --git a/drivers/staging/rtl8188eu/hal/odm.c b/drivers/staging/rtl8188eu/hal/odm.c
index d12eac4..25a4043 100644
--- a/drivers/staging/rtl8188eu/hal/odm.c
+++ b/drivers/staging/rtl8188eu/hal/odm.c
@@ -1042,6 +1042,7 @@ void odm_RefreshRateAdaptiveMaskCE(struct odm_dm_struct *pDM_Odm)

for (i = 0; i < ODM_ASSOCIATE_ENTRY_NUM; i++) {
struct sta_info *pstat = pDM_Odm->pODM_StaInfo[i];
+
if (IS_STA_VALID(pstat)) {
if (ODM_RAStateCheck(pDM_Odm, pstat->rssi_stat.UndecoratedSmoothedPWDB, false, &pstat->rssi_level)) {
ODM_RT_TRACE(pDM_Odm, ODM_COMP_RA_MASK, ODM_DBG_LOUD,
@@ -1107,6 +1108,7 @@ void odm_DynamicTxPowerInit(struct odm_dm_struct *pDM_Odm)
struct adapter *Adapter = pDM_Odm->Adapter;
struct hal_data_8188e *pHalData = GET_HAL_DATA(Adapter);
struct dm_priv *pdmpriv = &pHalData->dmpriv;
+
pdmpriv->bDynamicTxPowerEnable = false;
pdmpriv->LastDTPLvl = TxHighPwrLevel_Normal;
pdmpriv->DynamicTxHighPowerLvl = TxHighPwrLevel_Normal;
@@ -1269,6 +1271,7 @@ void odm_HwAntDiv(struct odm_dm_struct *pDM_Odm)
void ODM_EdcaTurboInit(struct odm_dm_struct *pDM_Odm)
{
struct adapter *Adapter = pDM_Odm->Adapter;
+
pDM_Odm->DM_EDCA_Table.bCurrentTurboEDCA = false;
pDM_Odm->DM_EDCA_Table.bIsCurRDLState = false;
Adapter->recvpriv.bIsAnyNonBEPkts = false;
diff --git a/drivers/staging/rtl8188eu/hal/odm_RTL8188E.c b/drivers/staging/rtl8188eu/hal/odm_RTL8188E.c
index fd7930f..57657aa 100644
--- a/drivers/staging/rtl8188eu/hal/odm_RTL8188E.c
+++ b/drivers/staging/rtl8188eu/hal/odm_RTL8188E.c
@@ -252,6 +252,7 @@ void rtl88eu_dm_ant_sel_statistics(struct odm_dm_struct *dm_odm,
u8 antsel_tr_mux, u32 mac_id, u8 rx_pwdb_all)
{
struct fast_ant_train *dm_fat_tbl = &dm_odm->DM_FatTable;
+
if (dm_odm->AntDivType == CG_TRX_HW_ANTDIV) {
if (antsel_tr_mux == MAIN_ANT_CG_TRX) {
dm_fat_tbl->MainAnt_Sum[mac_id] += rx_pwdb_all;
diff --git a/drivers/staging/rtl8188eu/hal/phy.c b/drivers/staging/rtl8188eu/hal/phy.c
index 0a1a7d6..40aa0ab 100644
--- a/drivers/staging/rtl8188eu/hal/phy.c
+++ b/drivers/staging/rtl8188eu/hal/phy.c
@@ -1187,6 +1187,7 @@ static void phy_iq_calibrate(struct adapter *adapt, s32 result[][8],
rFPGA0_XB_RFInterfaceOE, rFPGA0_RFMOD};

u32 retry_count = 9;
+
if (*(dm_odm->mp_mode) == 1)
retry_count = 9;
else
diff --git a/drivers/staging/rtl8188eu/hal/rtl8188e_cmd.c b/drivers/staging/rtl8188eu/hal/rtl8188e_cmd.c
index cf2609f..e8015ae 100644
--- a/drivers/staging/rtl8188eu/hal/rtl8188e_cmd.c
+++ b/drivers/staging/rtl8188eu/hal/rtl8188e_cmd.c
@@ -234,6 +234,7 @@ void rtl8188e_set_FwMediaStatus_cmd(struct adapter *adapt, __le16 mstatus_rpt)
{
u8 opmode, macid;
u16 mst_rpt = le16_to_cpu(mstatus_rpt);
+
opmode = (u8)mst_rpt;
macid = (u8)(mst_rpt >> 8);

diff --git a/drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c b/drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c
index c727a78..9318eb8 100644
--- a/drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c
+++ b/drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c
@@ -75,6 +75,7 @@ s32 iol_execute(struct adapter *padapter, u8 control)
static s32 iol_InitLLTTable(struct adapter *padapter, u8 txpktbuf_bndy)
{
s32 rst = _SUCCESS;
+
iol_mode_enable(padapter, 1);
usb_write8(padapter, REG_TDECTRL+1, txpktbuf_bndy);
rst = iol_execute(padapter, CMD_INIT_LLT);
@@ -178,6 +179,7 @@ static void rtl8188e_SetHalODMVar(struct adapter *Adapter, enum hal_odm_variable
{
struct hal_data_8188e *pHalData = GET_HAL_DATA(Adapter);
struct odm_dm_struct *podmpriv = &pHalData->odmpriv;
+
switch (eVariable) {
case HAL_ODM_STA_INFO:
{
diff --git a/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c b/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c
index 594c1da..9defac8 100644
--- a/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c
+++ b/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c
@@ -39,8 +39,8 @@ static u8 urb_zero_packet_chk(struct adapter *adapt, int sz)
{
u8 set_tx_desc_offset;
struct hal_data_8188e *haldata = GET_HAL_DATA(adapt);
- set_tx_desc_offset = (((sz + TXDESC_SIZE) % haldata->UsbBulkOutSize) == 0) ? 1 : 0;

+ set_tx_desc_offset = (((sz + TXDESC_SIZE) % haldata->UsbBulkOutSize) == 0) ? 1 : 0;
return set_tx_desc_offset;
}

@@ -356,6 +356,7 @@ static s32 rtw_dump_xframe(struct adapter *adapt, struct xmit_frame *pxmitframe)
struct pkt_attrib *pattrib = &pxmitframe->attrib;
struct xmit_priv *pxmitpriv = &adapt->xmitpriv;
struct security_priv *psecuritypriv = &adapt->securitypriv;
+
if ((pxmitframe->frame_tag == DATA_FRAMETAG) &&
(pxmitframe->attrib.ether_type != 0x0806) &&
(pxmitframe->attrib.ether_type != 0x888e) &&
diff --git a/drivers/staging/rtl8188eu/hal/usb_halinit.c b/drivers/staging/rtl8188eu/hal/usb_halinit.c
index 5665a7d..78ebc77 100644
--- a/drivers/staging/rtl8188eu/hal/usb_halinit.c
+++ b/drivers/staging/rtl8188eu/hal/usb_halinit.c
@@ -104,6 +104,7 @@ static u32 rtl8188eu_InitPowerOn(struct adapter *adapt)
u16 value16;
/* HW Power on sequence */
struct hal_data_8188e *haldata = GET_HAL_DATA(adapt);
+
if (haldata->bMacPwrCtrlOn)
return _SUCCESS;

@@ -232,8 +233,8 @@ static void _InitNormalChipRegPriority(struct adapter *Adapter, u16 beQ,
static void _InitNormalChipOneOutEpPriority(struct adapter *Adapter)
{
struct hal_data_8188e *haldata = GET_HAL_DATA(Adapter);
-
u16 value = 0;
+
switch (haldata->OutEpQueueSel) {
case TX_SELE_HQ:
value = QUEUE_HIGH;
@@ -350,8 +351,8 @@ static void _InitNetworkType(struct adapter *Adapter)
static void _InitTransferPageSize(struct adapter *Adapter)
{
/* Tx page size is always 128. */
-
u8 value8;
+
value8 = _PSRX(PBP_128) | _PSTX(PBP_128);
usb_write8(Adapter, REG_PBP, value8);
}
@@ -1416,6 +1417,7 @@ static void SetHwReg8188EU(struct adapter *Adapter, u8 variable, u8 *val)
if (*((u8 *)val)) { /* under sitesurvey */
/* config RCR to receive different BSSID & not to receive data frame */
u32 v = usb_read32(Adapter, REG_RCR);
+
v &= ~(RCR_CBSSID_BCN);
usb_write32(Adapter, REG_RCR, v);
/* reject all data frame */
@@ -1444,6 +1446,7 @@ static void SetHwReg8188EU(struct adapter *Adapter, u8 variable, u8 *val)
} else {
if (Adapter->in_cta_test) {
u32 v = usb_read32(Adapter, REG_RCR);
+
v &= ~(RCR_CBSSID_DATA | RCR_CBSSID_BCN);/* RCR_ADF */
usb_write32(Adapter, REG_RCR, v);
} else {
@@ -1464,6 +1467,7 @@ static void SetHwReg8188EU(struct adapter *Adapter, u8 variable, u8 *val)

if (Adapter->in_cta_test) {
u32 v = usb_read32(Adapter, REG_RCR);
+
v &= ~(RCR_CBSSID_DATA | RCR_CBSSID_BCN);/* RCR_ADF */
usb_write32(Adapter, REG_RCR, v);
} else {
@@ -1588,8 +1592,8 @@ static void SetHwReg8188EU(struct adapter *Adapter, u8 variable, u8 *val)
{
u32 cmd;
u32 *cam_val = (u32 *)val;
- usb_write32(Adapter, WCAMI, cam_val[0]);

+ usb_write32(Adapter, WCAMI, cam_val[0]);
cmd = CAM_POLLINIG | CAM_WRITE | cam_val[1];
usb_write32(Adapter, RWCAM, cmd);
}
@@ -1691,6 +1695,7 @@ static void SetHwReg8188EU(struct adapter *Adapter, u8 variable, u8 *val)
case HW_VAR_RXDMA_AGG_PG_TH:
{
u8 threshold = *((u8 *)val);
+
if (threshold == 0)
threshold = haldata->UsbRxAggPageCount;
usb_write8(Adapter, REG_RXDMA_AGG_PG_TH, threshold);
@@ -1712,6 +1717,7 @@ static void SetHwReg8188EU(struct adapter *Adapter, u8 variable, u8 *val)
case HW_VAR_H2C_FW_JOINBSSRPT:
{
u8 mstatus = (*(u8 *)val);
+
rtl8188e_set_FwJoinBssReport_cmd(Adapter, mstatus);
}
break;
@@ -1734,6 +1740,7 @@ static void SetHwReg8188EU(struct adapter *Adapter, u8 variable, u8 *val)
case HW_VAR_RPT_TIMER_SETTING:
{
u16 min_rpt_time = (*(u16 *)val);
+
ODM_RA_Set_TxRPT_Time(podmpriv, min_rpt_time);
}
break;
@@ -1790,6 +1797,7 @@ static void SetHwReg8188EU(struct adapter *Adapter, u8 variable, u8 *val)
case HW_VAR_TX_RPT_MAX_MACID:
{
u8 maxMacid = *val;
+
DBG_88E("### MacID(%d),Set Max Tx RPT MID(%d)\n", maxMacid, maxMacid+1);
usb_write8(Adapter, REG_TX_RPT_CTRL+1, maxMacid+1);
}
@@ -1836,6 +1844,7 @@ static void GetHwReg8188EU(struct adapter *Adapter, u8 variable, u8 *val)
val[0] = true;
} else {
u32 valRCR;
+
valRCR = usb_read32(Adapter, REG_RCR);
valRCR &= 0x00070000;
if (valRCR)
@@ -1883,6 +1892,7 @@ GetHalDefVar8188EUsb(
struct mlme_priv *pmlmepriv = &Adapter->mlmepriv;
struct sta_priv *pstapriv = &Adapter->stapriv;
struct sta_info *psta;
+
psta = rtw_get_stainfo(pstapriv, pmlmepriv->cur_network.network.MacAddress);
if (psta)
*((int *)pValue) = psta->rssi_stat.UndecoratedSmoothedPWDB;
@@ -1930,6 +1940,7 @@ GetHalDefVar8188EUsb(
case HW_DEF_RA_INFO_DUMP:
{
u8 entry_id = *((u8 *)pValue);
+
if (check_fwstate(&Adapter->mlmepriv, _FW_LINKED)) {
DBG_88E("============ RA status check ===================\n");
DBG_88E("Mac_id:%d , RateID = %d, RAUseRate = 0x%08x, RateSGI = %d, DecisionRate = 0x%02x ,PTStage = %d\n",
@@ -1945,6 +1956,7 @@ GetHalDefVar8188EUsb(
case HW_DEF_ODM_DBG_FLAG:
{
struct odm_dm_struct *dm_ocm = &(haldata->odmpriv);
+
pr_info("dm_ocm->DebugComponents = 0x%llx\n", dm_ocm->DebugComponents);
}
break;
@@ -1982,6 +1994,7 @@ static u8 SetHalDefVar8188EUsb(struct adapter *Adapter, enum hal_def_variable eV
DBG_88E("==> Disable all dynamic function...\n");
} else if (dm_func == 1) {/* disable DIG */
podmpriv->SupportAbility &= (~DYNAMIC_BB_DIG);
+
DBG_88E("==> Disable DIG...\n");
} else if (dm_func == 2) {/* disable High power */
podmpriv->SupportAbility &= (~DYNAMIC_BB_DYNAMIC_TXPWR);
@@ -1993,6 +2006,7 @@ static u8 SetHalDefVar8188EUsb(struct adapter *Adapter, enum hal_def_variable eV
} else if (dm_func == 6) {/* turn on all dynamic func */
if (!(podmpriv->SupportAbility & DYNAMIC_BB_DIG)) {
struct rtw_dig *pDigTable = &podmpriv->DM_DigTable;
+
pDigTable->CurIGValue = usb_read8(Adapter, 0xc50);
}
podmpriv->SupportAbility = DYNAMIC_ALL_FUNC_ENABLE;
@@ -2010,6 +2024,7 @@ static u8 SetHalDefVar8188EUsb(struct adapter *Adapter, enum hal_def_variable eV
{
u8 bRSSIDump = *((u8 *)pValue);
struct odm_dm_struct *dm_ocm = &(haldata->odmpriv);
+
if (bRSSIDump)
dm_ocm->DebugComponents = ODM_COMP_DIG|ODM_COMP_FA_CNT;
else
@@ -2020,6 +2035,7 @@ static u8 SetHalDefVar8188EUsb(struct adapter *Adapter, enum hal_def_variable eV
{
u64 DebugComponents = *((u64 *)pValue);
struct odm_dm_struct *dm_ocm = &(haldata->odmpriv);
+
dm_ocm->DebugComponents = DebugComponents;
}
break;
--
1.9.1

2015-02-10 13:02:01

by Pushpendra Singh

[permalink] [raw]
Subject: [PATCH 5/6] staging: rtl8188eu: hal: removed spaces at start of line

Removed checkpatch.pl below warning
WARNING: please, no spaces at the start of a line

Signed-off-by: Pushpendra Singh <[email protected]>
---
drivers/staging/rtl8188eu/hal/usb_halinit.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8188eu/hal/usb_halinit.c b/drivers/staging/rtl8188eu/hal/usb_halinit.c
index 78ebc77..7714f5f 100644
--- a/drivers/staging/rtl8188eu/hal/usb_halinit.c
+++ b/drivers/staging/rtl8188eu/hal/usb_halinit.c
@@ -1004,7 +1004,7 @@ static u32 rtl8188eu_hal_deinit(struct adapter *Adapter)
}
}
return _SUCCESS;
- }
+}

static unsigned int rtl8188eu_inirp_init(struct adapter *Adapter)
{
--
1.9.1

2015-02-10 13:02:10

by Pushpendra Singh

[permalink] [raw]
Subject: [PATCH 6/6] staging: rtl8188eu: hal: removed unnecessary parentheses

Removed checkpatch.pl below warning
WARNING: Unnecessary parentheses - maybe == should be = ?

Signed-off-by: Pushpendra Singh <[email protected]>
---
drivers/staging/rtl8188eu/hal/usb_halinit.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8188eu/hal/usb_halinit.c b/drivers/staging/rtl8188eu/hal/usb_halinit.c
index 7714f5f..274f4ac 100644
--- a/drivers/staging/rtl8188eu/hal/usb_halinit.c
+++ b/drivers/staging/rtl8188eu/hal/usb_halinit.c
@@ -1218,7 +1218,7 @@ static void hw_var_set_opmode(struct adapter *Adapter, u8 variable, u8 *val)
StopTxBeacon(Adapter);

usb_write8(Adapter, REG_BCN_CTRL, 0x19);/* disable atim wnd */
- } else if ((mode == _HW_STATE_ADHOC_)) {
+ } else if (mode == _HW_STATE_ADHOC_) {
ResumeTxBeacon(Adapter);
usb_write8(Adapter, REG_BCN_CTRL, 0x1a);
} else if (mode == _HW_STATE_AP_) {
--
1.9.1

2015-02-10 14:13:16

by Sudip Mukherjee

[permalink] [raw]
Subject: Re: [PATCH 2/6] staging: rtl8188eu: hal: removed code indent error

On Tue, Feb 10, 2015 at 06:32:49PM +0530, Pushpendra Singh wrote:
> Removed checkpatch.pl error
> ERROR: code indent should use tabs where possible
>
> Signed-off-by: Pushpendra Singh <[email protected]>
> ---
> drivers/staging/rtl8188eu/hal/rf.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/staging/rtl8188eu/hal/rf.c b/drivers/staging/rtl8188eu/hal/rf.c
> index eea4c8a..88a026c 100644
> --- a/drivers/staging/rtl8188eu/hal/rf.c
> +++ b/drivers/staging/rtl8188eu/hal/rf.c
> @@ -101,8 +101,7 @@ void rtl88eu_phy_rf6052_set_cck_txpower(struct adapter *adapt, u8 *powerlevel)
> ptr++;
> }
> }
> - rtl88eu_dm_txpower_track_adjust(&hal_data->odmpriv, 1, &direction,
> - &pwrtrac_value);
> + rtl88eu_dm_txpower_track_adjust(&hal_data->odmpriv, 1, &direction, &pwrtrac_value);
you are introducing one warning to fix one error. line over 80 character.

sudip
>
> if (direction == 1) {
> /* Increase TX power */
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2015-02-10 14:27:13

by Bas Peters

[permalink] [raw]
Subject: Re: [PATCH 2/6] staging: rtl8188eu: hal: removed code indent error

>> @@ -101,8 +101,7 @@ void rtl88eu_phy_rf6052_set_cck_txpower(struct adapter *adapt, u8 *powerlevel)
>> ptr++;
>> }
>> }
>> - rtl88eu_dm_txpower_track_adjust(&hal_data->odmpriv, 1, &direction,
>> - &pwrtrac_value);
>> + rtl88eu_dm_txpower_track_adjust(&hal_data->odmpriv, 1, &direction, &pwrtrac_value);
> you are introducing one warning to fix one error. line over 80 character.

Isn't that warning more of a guideline, rather than an actual warning?
This seems more readable than:

- rtl88eu_dm_txpower_track_adjust(&hal_data->odmpriv, 1, &direction,
- &pwrtrac_value);
+ rtl88eu_dm_txpower_track_adjust(&hal_data->odmpriv, 1,
+
&direction, &pwrtrac_value);

Bas

2015-02-10 14:48:54

by Sudip Mukherjee

[permalink] [raw]
Subject: Re: [PATCH 2/6] staging: rtl8188eu: hal: removed code indent error

On Tue, Feb 10, 2015 at 03:27:11PM +0100, Bas Peters wrote:
> >> @@ -101,8 +101,7 @@ void rtl88eu_phy_rf6052_set_cck_txpower(struct adapter *adapt, u8 *powerlevel)
> >> ptr++;
> >> }
> >> }
> >> - rtl88eu_dm_txpower_track_adjust(&hal_data->odmpriv, 1, &direction,
> >> - &pwrtrac_value);
> >> + rtl88eu_dm_txpower_track_adjust(&hal_data->odmpriv, 1, &direction, &pwrtrac_value);
> > you are introducing one warning to fix one error. line over 80 character.
>
> Isn't that warning more of a guideline, rather than an actual warning?
> This seems more readable than:
>
> - rtl88eu_dm_txpower_track_adjust(&hal_data->odmpriv, 1, &direction,
> - &pwrtrac_value);
> + rtl88eu_dm_txpower_track_adjust(&hal_data->odmpriv, 1,
> +
> &direction, &pwrtrac_value);
CodingStyle says "Statements longer than 80 columns will be broken into sensible chunks,
unless exceeding 80 columns significantly increases readability and does not hide information"

i agree that this is more readable but can it be called significant improvement ? but ofcourse final decision is on Greg.

sudip
>
> Bas

2015-02-10 22:25:50

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 2/6] staging: rtl8188eu: hal: removed code indent error

On Tue, Feb 10, 2015 at 06:32:49PM +0530, Pushpendra Singh wrote:
> Removed checkpatch.pl error
> ERROR: code indent should use tabs where possible

Then fix the indentation with a tab, not using over 8 spaces, don't make
this a new coding style issue that I will then get an additional patch
for later from someone that will just fix up the original tab issue...

thanks,

greg k-h

2015-02-11 08:33:40

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH 2/6] staging: rtl8188eu: hal: removed code indent error

On Tue, Feb 10, 2015 at 03:27:11PM +0100, Bas Peters wrote:
> >> @@ -101,8 +101,7 @@ void rtl88eu_phy_rf6052_set_cck_txpower(struct adapter *adapt, u8 *powerlevel)
> >> ptr++;
> >> }
> >> }
> >> - rtl88eu_dm_txpower_track_adjust(&hal_data->odmpriv, 1, &direction,
> >> - &pwrtrac_value);
> >> + rtl88eu_dm_txpower_track_adjust(&hal_data->odmpriv, 1, &direction, &pwrtrac_value);
> > you are introducing one warning to fix one error. line over 80 character.
>
> Isn't that warning more of a guideline, rather than an actual warning?

You can't fight checkpatch.pl. We reject the worst or the worst "break
lines up into smaller chunks" patches where they obviously hurt
readability, but we almost always silence the warning in the end. The
original code in this case was fine.

regards,
dan carpenter

2015-02-11 09:40:43

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH 2/6] staging: rtl8188eu: hal: removed code indent error

On Wed, 2015-02-11 at 11:33 +0300, Dan Carpenter wrote:
> On Tue, Feb 10, 2015 at 03:27:11PM +0100, Bas Peters wrote:
> > >> @@ -101,8 +101,7 @@ void rtl88eu_phy_rf6052_set_cck_txpower(struct adapter *adapt, u8 *powerlevel)
> > >> ptr++;
> > >> }
> > >> }
> > >> - rtl88eu_dm_txpower_track_adjust(&hal_data->odmpriv, 1, &direction,
> > >> - &pwrtrac_value);
> > >> + rtl88eu_dm_txpower_track_adjust(&hal_data->odmpriv, 1, &direction, &pwrtrac_value);
> > > you are introducing one warning to fix one error. line over 80 character.
> >
> > Isn't that warning more of a guideline, rather than an actual warning?

Yes, it is more informational than defect.

> You can't fight checkpatch.pl.

Sure you can, Ignore it whenever appropriate.

It's a pity there are _so_ many people that think
checkpatch messages are gospel.

> We reject the worst or the worst "break
> lines up into smaller chunks" patches where they obviously hurt
> readability, but we almost always silence the warning in the end. The
> original code in this case was fine.

Any line with 30+ char identifiers generally doesn't
fit well in 80 char lines.

2015-02-11 10:51:28

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH 2/6] staging: rtl8188eu: hal: removed code indent error

On Wed, Feb 11, 2015 at 01:40:37AM -0800, Joe Perches wrote:
> On Wed, 2015-02-11 at 11:33 +0300, Dan Carpenter wrote:
> > You can't fight checkpatch.pl.
>
> Sure you can, Ignore it whenever appropriate.

People will just keep sending patches until something gets merged.

It's rude to ignore patches and it's useless because people will just
send another email asking you "have you received my patch yet?". It
just creates a bigger fight.

Applying mediocre checkpatch cleanups takes less time and energy than
constantly fighting. It's easiest to not fight over stupid stuff and
just apply the patches. Plus it makes the patch senders happy and
that creates a happier community.

regards,
dan carpenter

2015-02-11 18:00:36

by Joe Perches

[permalink] [raw]
Subject: checkpatch induced patches...

On Wed, 2015-02-11 at 13:51 +0300, Dan Carpenter wrote:
> On Wed, Feb 11, 2015 at 01:40:37AM -0800, Joe Perches wrote:
> > On Wed, 2015-02-11 at 11:33 +0300, Dan Carpenter wrote:
> > > You can't fight checkpatch.pl.
> >
> > Sure you can, Ignore it whenever appropriate.
>
> People will just keep sending patches until something gets merged.
>
> It's rude to ignore patches and it's useless because people will just
> send another email asking you "have you received my patch yet?". It
> just creates a bigger fight.

> Applying mediocre checkpatch cleanups takes less time and energy than
> constantly fighting.

Mediocre cleanup patches that fall into the
"not satisfactory, poor, inferior" category
shouldn't be applied.

> It's easiest to not fight over stupid stuff and
> just apply the patches. Plus it makes the patch senders happy and
> that creates a happier community.

The primary thing I'd like to see stopped is the
use of checkpatch to satisfy some CS assignment.

Have any of those submitters ever gone on to produce
more thorough patches?

I'm half tempted to submit some patch like this to
make it difficult to use checkpatch on files outside
of drivers/staging.

o Only allow checkpatch to be used with the -f/--file
option for drivers/staging/
o Add an undocumented --force command line option
o Make --strict the default for drivers/staging
---
scripts/checkpatch.pl | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 3642b0d..70f1047 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -25,6 +25,7 @@ my $tst_only;
my $emacs = 0;
my $terse = 0;
my $file = 0;
+my $force = 0;
my $check = 0;
my $check_orig = 0;
my $summary = 1;
@@ -130,6 +131,7 @@ GetOptions(
'emacs!' => \$emacs,
'terse!' => \$terse,
'f|file!' => \$file,
+ 'force!' => \$force,
'subjective!' => \$check,
'strict!' => \$check,
'ignore=s' => \@ignore,
@@ -674,6 +676,10 @@ my $fixlinenr = -1;
my $vname;
for my $filename (@ARGV) {
my $FILE;
+ if (!$force && $file && $filename !~ m@^drivers/staging/@) {
+ warn "$P: checking '$filename' is not supported\n";
+ next;
+ }
if ($file) {
open($FILE, '-|', "diff -u /dev/null $filename") ||
die "$P: $filename: diff failed - $!\n";
@@ -2062,7 +2068,7 @@ sub process {
}

if ($found_file) {
- if ($realfile =~ m@^(drivers/net/|net/)@) {
+ if ($realfile =~ m@^(?:drivers/net/|net/|drivers/staging/)@) {
$check = 1;
} else {
$check = $check_orig;


2015-02-11 18:36:29

by Dan Carpenter

[permalink] [raw]
Subject: Re: checkpatch induced patches...

On Wed, Feb 11, 2015 at 10:00:29AM -0800, Joe Perches wrote:
> I'm half tempted to submit some patch like this to
> make it difficult to use checkpatch on files outside
> of drivers/staging.
>
> o Only allow checkpatch to be used with the -f/--file
> option for drivers/staging/
> o Add an undocumented --force command line option

Sure. We could try that. I once sent a patch to make -f generate a
warning about not wasting people's time, but this is also ok.

> o Make --strict the default for drivers/staging

Ack.

regards,
dan carpenter

2015-02-11 20:02:33

by Richard Weinberger

[permalink] [raw]
Subject: Re: checkpatch induced patches...

On Wed, Feb 11, 2015 at 7:36 PM, Dan Carpenter <[email protected]> wrote:
> On Wed, Feb 11, 2015 at 10:00:29AM -0800, Joe Perches wrote:
>> I'm half tempted to submit some patch like this to
>> make it difficult to use checkpatch on files outside
>> of drivers/staging.
>>
>> o Only allow checkpatch to be used with the -f/--file
>> option for drivers/staging/
>> o Add an undocumented --force command line option
>
> Sure. We could try that. I once sent a patch to make -f generate a
> warning about not wasting people's time, but this is also ok.
>
>> o Make --strict the default for drivers/staging
>
> Ack.

FYI: We had already a heated debate on that topic.
https://lkml.org/lkml/2014/7/17/415

--
Thanks,
//richard

2015-02-11 20:20:33

by Joe Perches

[permalink] [raw]
Subject: Re: checkpatch induced patches...

On Wed, 2015-02-11 at 21:02 +0100, Richard Weinberger wrote:
> On Wed, Feb 11, 2015 at 7:36 PM, Dan Carpenter <[email protected]> wrote:
> > On Wed, Feb 11, 2015 at 10:00:29AM -0800, Joe Perches wrote:
> >> I'm half tempted to submit some patch like this to
> >> make it difficult to use checkpatch on files outside
> >> of drivers/staging.
> >>
> >> o Only allow checkpatch to be used with the -f/--file
> >> option for drivers/staging/
> >> o Add an undocumented --force command line option
> >
> > Sure. We could try that. I once sent a patch to make -f generate a
> > warning about not wasting people's time, but this is also ok.
> >
> >> o Make --strict the default for drivers/staging
> >
> > Ack.
>
> FYI: We had already a heated debate on that topic.
> https://lkml.org/lkml/2014/7/17/415

Yeah, I remember.

It's always a pleasure to chat with Borislav.

This is basically a patch that implements my suggestion
in that thread.

https://lkml.org/lkml/2014/7/17/427

I wonder if the undocumented --force option is acceptable
to Pavel and Kalle.

2015-02-11 20:24:44

by Pavel Machek

[permalink] [raw]
Subject: Re: checkpatch induced patches...

On Wed 2015-02-11 12:20:25, Joe Perches wrote:
> On Wed, 2015-02-11 at 21:02 +0100, Richard Weinberger wrote:
> > On Wed, Feb 11, 2015 at 7:36 PM, Dan Carpenter <[email protected]> wrote:
> > > On Wed, Feb 11, 2015 at 10:00:29AM -0800, Joe Perches wrote:
> > >> I'm half tempted to submit some patch like this to
> > >> make it difficult to use checkpatch on files outside
> > >> of drivers/staging.
> > >>
> > >> o Only allow checkpatch to be used with the -f/--file
> > >> option for drivers/staging/
> > >> o Add an undocumented --force command line option
> > >
> > > Sure. We could try that. I once sent a patch to make -f generate a
> > > warning about not wasting people's time, but this is also ok.
> > >
> > >> o Make --strict the default for drivers/staging
> > >
> > > Ack.
> >
> > FYI: We had already a heated debate on that topic.
> > https://lkml.org/lkml/2014/7/17/415
>
> Yeah, I remember.
>
> It's always a pleasure to chat with Borislav.
>
> This is basically a patch that implements my suggestion
> in that thread.
>
> https://lkml.org/lkml/2014/7/17/427
>
> I wonder if the undocumented --force option is acceptable
> to Pavel and Kalle.

Undocumented options are evil... You can add warning about not wasting
people's time in --force documentation...

Pavel

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2015-02-11 20:43:10

by Joe Perches

[permalink] [raw]
Subject: Re: checkpatch induced patches...

On Wed, 2015-02-11 at 21:24 +0100, Pavel Machek wrote:
> On Wed 2015-02-11 12:20:25, Joe Perches wrote:
> > On Wed, 2015-02-11 at 21:02 +0100, Richard Weinberger wrote:
> > > On Wed, Feb 11, 2015 at 7:36 PM, Dan Carpenter <[email protected]> wrote:
> > > > On Wed, Feb 11, 2015 at 10:00:29AM -0800, Joe Perches wrote:
> > > >> I'm half tempted to submit some patch like this to
> > > >> make it difficult to use checkpatch on files outside
> > > >> of drivers/staging.
> > > >> o Only allow checkpatch to be used with the -f/--file
> > > >> option for drivers/staging/
> > > >> o Add an undocumented --force command line option
> > > > Sure. We could try that. I once sent a patch to make -f generate a
> > > > warning about not wasting people's time, but this is also ok.
> > > >> o Make --strict the default for drivers/staging
[]
> > > FYI: We had already a heated debate on that topic.
> > > https://lkml.org/lkml/2014/7/17/415
[]
> > This is basically a patch that implements my suggestion
> > in that thread.
> > https://lkml.org/lkml/2014/7/17/427
> >
> > I wonder if the undocumented --force option is acceptable
> > to Pavel and Kalle.

> Undocumented options are evil... You can add warning about not wasting
> people's time in --force documentation...

Yeah, I had added --force to the help text
then removed it before sending, so I suppose
adding a warning there is OK too.

Nobody reads the --help text anyway.

Dan/Andrew/Greg? You got a preference?

Maybe some help/warning text like:

--force Without --force, checkpatch will not scan files
using -f or --file outside of drivers/staging/...
Do not use this option merely to create potential
patches that are uncompiled or untested.

2015-02-11 22:43:13

by Dan Carpenter

[permalink] [raw]
Subject: Re: checkpatch induced patches...

On Wed, Feb 11, 2015 at 12:43:03PM -0800, Joe Perches wrote:
> Maybe some help/warning text like:
>
> --force Without --force, checkpatch will not scan files
> using -f or --file outside of drivers/staging/...
> Do not use this option merely to create potential
> patches that are uncompiled or untested.

Everyone compiles their patches hopefully? The problem is with patches
that aren't really a cleanup but are just done to make checkpatch happy.

I guess documenting --force is better than not documenting.

regards,
dan carpenter

2015-02-11 22:47:06

by Richard Weinberger

[permalink] [raw]
Subject: Re: checkpatch induced patches...

Am 11.02.2015 um 23:43 schrieb Dan Carpenter:
> On Wed, Feb 11, 2015 at 12:43:03PM -0800, Joe Perches wrote:
>> Maybe some help/warning text like:
>>
>> --force Without --force, checkpatch will not scan files
>> using -f or --file outside of drivers/staging/...
>> Do not use this option merely to create potential
>> patches that are uncompiled or untested.
>
> Everyone compiles their patches hopefully? The problem is with patches
> that aren't really a cleanup but are just done to make checkpatch happy.
>
> I guess documenting --force is better than not documenting.

Documentation is like sex: when it is good, it is very, very good; and when it is bad, it is better than nothing. -- Dick Brandon

Thanks,
//richard

2015-02-11 23:01:45

by Joe Perches

[permalink] [raw]
Subject: Re: checkpatch induced patches...

On Thu, 2015-02-12 at 01:43 +0300, Dan Carpenter wrote:
> On Wed, Feb 11, 2015 at 12:43:03PM -0800, Joe Perches wrote:
> > Maybe some help/warning text like:
> >
> > --force Without --force, checkpatch will not scan files
> > using -f or --file outside of drivers/staging/...
> > Do not use this option merely to create potential
> > patches that are uncompiled or untested.
>
> Everyone compiles their patches hopefully?

Maybe they're simply hopeful their patches compile.

Many checkpatch users seem unaware their patches
need to compile though.

2015-02-12 08:04:11

by Sudip Mukherjee

[permalink] [raw]
Subject: Re: checkpatch induced patches...

On Thu, Feb 12, 2015 at 01:43:00AM +0300, Dan Carpenter wrote:
> On Wed, Feb 11, 2015 at 12:43:03PM -0800, Joe Perches wrote:
> > Maybe some help/warning text like:
> >
> > --force Without --force, checkpatch will not scan files
> > using -f or --file outside of drivers/staging/...
> > Do not use this option merely to create potential
> > patches that are uncompiled or untested.
>
> Everyone compiles their patches hopefully? The problem is with patches
> that aren't really a cleanup but are just done to make checkpatch happy.
>
> I guess documenting --force is better than not documenting.
can i make a suggestion?

1) we can have some sort of symbol in the MAINTAINER file to show if that maintainer wants style correction patch or not.
a) if the maintainer doesnot want to receive such patches then checkpatch will only check that patch if that patch is part of a series, and there should be an extra option in checkpatch so that the user can inform checkpatch that it is a part of a series which is doing more than just style cleanups.
b) And if the maintainer welcomes style check patches then checkpatch can check the patch without any extra option and -f can also be enabld for those files which are maintained by that particular maintainer.

2) is it not possible for checkpatch to check if the patch is already applied to the file or not? if the patch is applied and the timestamp of the .o file is older than the file in question then that will usually mean the user has not compiled the patch.

my perl skills are excellent so i am not sure these are practical suggestions or not .. :)

regards
sudip

>
> regards,
> dan carpenter

2015-02-12 16:44:32

by Kalle Valo

[permalink] [raw]
Subject: Re: checkpatch induced patches...

Joe Perches <[email protected]> writes:

> On Wed, 2015-02-11 at 21:02 +0100, Richard Weinberger wrote:
>> On Wed, Feb 11, 2015 at 7:36 PM, Dan Carpenter <[email protected]> wrote:
>> > On Wed, Feb 11, 2015 at 10:00:29AM -0800, Joe Perches wrote:
>> >> I'm half tempted to submit some patch like this to
>> >> make it difficult to use checkpatch on files outside
>> >> of drivers/staging.
>> >>
>> >> o Only allow checkpatch to be used with the -f/--file
>> >> option for drivers/staging/
>> >> o Add an undocumented --force command line option
>> >
>> > Sure. We could try that. I once sent a patch to make -f generate a
>> > warning about not wasting people's time, but this is also ok.
>> >
>> >> o Make --strict the default for drivers/staging
>> >
>> > Ack.
>>
>> FYI: We had already a heated debate on that topic.
>> https://lkml.org/lkml/2014/7/17/415
>
> Yeah, I remember.
>
> It's always a pleasure to chat with Borislav.
>
> This is basically a patch that implements my suggestion
> in that thread.
>
> https://lkml.org/lkml/2014/7/17/427
>
> I wonder if the undocumented --force option is acceptable
> to Pavel and Kalle.

I don't mind if I have to add --force to my scripts as long as
checkpatch works similarly as before.

--
Kalle Valo