Skip to contentSkip to conversation

Staging: rtl8192e: Rename variable pBA

PATCH In review Tree Davies 3 participants 6 messages Jul 12, 2023 – Jul 14, 2023 Staging

lore
Share & tools
lore

Tree Davies

author
#0 lore raw

PATCH 9/12 Staging: rtl8192e: Rename variable pBA

Rename variable pBA to pba in order to Fix checkpatch
warning: Avoid CamelCase

Signed-off-by Tree Davies <[email protected]>
---
 drivers/staging/rtl8192e/rtl819x_BAProc.c | 106 +++++++++++-----------
 drivers/staging/rtl8192e/rtllib.h         |   2 +-
 2 files changed, 54 insertions(+), 54 deletions(-)
drivers/staging/rtl8192e/rtl819x_BAProc.c +53 -53
index 6b5da38353ee..43ee1bd4a6ed 100644
@@ -10,17 +10,17 @@
 #include "rtllib.h"
 #include "rtl819x_BA.h"
 
-static void activate_ba_entry(struct ba_record *pBA, u16 Time)
+static void activate_ba_entry(struct ba_record *pba, u16 Time)
 {
-	pBA->b_valid = true;
+	pba->b_valid = true;
 	if (Time != 0)
-		mod_timer(&pBA->timer, jiffies + msecs_to_jiffies(Time));
+		mod_timer(&pba->timer, jiffies + msecs_to_jiffies(Time));
 }
 
-static void deactivate_ba_entry(struct rtllib_device *ieee, struct ba_record *pBA)
+static void deactivate_ba_entry(struct rtllib_device *ieee, struct ba_record *pba)
 {
-	pBA->b_valid = false;
-	del_timer_sync(&pBA->timer);
+	pba->b_valid = false;
+	del_timer_sync(&pba->timer);
 }
 
 static u8 tx_ts_delete_ba(struct rtllib_device *ieee, struct tx_ts_record *pTxTs)

@@ -43,28 +43,28 @@ static u8 tx_ts_delete_ba(struct rtllib_device *ieee, struct tx_ts_record *pTxTs
 
 static u8 RxTsDeleteBA(struct rtllib_device *ieee, struct rx_ts_record *pRxTs)
 {
-	struct ba_record *pBa = &pRxTs->rx_admitted_ba_record;
+	struct ba_record *pba = &pRxTs->rx_admitted_ba_record;
 	u8			bSendDELBA = false;
 
-	if (pBa->b_valid) {
-		deactivate_ba_entry(ieee, pBa);
+	if (pba->b_valid) {
+		deactivate_ba_entry(ieee, pba);
 		bSendDELBA = true;
 	}
 
 	return bSendDELBA;
 }
 
-void ResetBaEntry(struct ba_record *pBA)
+void ResetBaEntry(struct ba_record *pba)
 {
-	pBA->b_valid			  = false;
-	pBA->ba_param_set.short_data	  = 0;
-	pBA->ba_timeout_value		  = 0;
-	pBA->dialog_token		  = 0;
-	pBA->ba_start_seq_ctrl.short_data = 0;
+	pba->b_valid			  = false;
+	pba->ba_param_set.short_data	  = 0;
+	pba->ba_timeout_value		  = 0;
+	pba->dialog_token		  = 0;
+	pba->ba_start_seq_ctrl.short_data = 0;
 }
 
 static struct sk_buff *rtllib_ADDBA(struct rtllib_device *ieee, u8 *Dst,
-				    struct ba_record *pBA,
+				    struct ba_record *pba,
 				    u16 StatusCode, u8 type)
 {
 	struct sk_buff *skb = NULL;

@@ -75,8 +75,8 @@ static struct sk_buff *rtllib_ADDBA(struct rtllib_device *ieee, u8 *Dst,
 	netdev_dbg(ieee->dev, "%s(): frame(%d) sentd to: %pM, ieee->dev:%p\n",
 		   __func__, type, Dst, ieee->dev);
 
-	if (!pBA) {
-		netdev_warn(ieee->dev, "pBA is NULL\n");
+	if (!pba) {
+		netdev_warn(ieee->dev, "pba is NULL\n");
 		return NULL;
 	}
 	skb = dev_alloc_skb(len + sizeof(struct rtllib_hdr_3addr));

@@ -98,21 +98,21 @@ static struct sk_buff *rtllib_ADDBA(struct rtllib_device *ieee, u8 *Dst,
 	tag = skb_put(skb, 9);
 	*tag++ = ACT_CAT_BA;
 	*tag++ = type;
-	*tag++ = pBA->dialog_token;
+	*tag++ = pba->dialog_token;
 
 	if (type == ACT_ADDBARSP) {
 		put_unaligned_le16(StatusCode, tag);
 		tag += 2;
 	}
 
-	put_unaligned_le16(pBA->ba_param_set.short_data, tag);
+	put_unaligned_le16(pba->ba_param_set.short_data, tag);
 	tag += 2;
 
-	put_unaligned_le16(pBA->ba_timeout_value, tag);
+	put_unaligned_le16(pba->ba_timeout_value, tag);
 	tag += 2;
 
 	if (type == ACT_ADDBAREQ) {
-		memcpy(tag, (u8 *)&pBA->ba_start_seq_ctrl, 2);
+		memcpy(tag, (u8 *)&pba->ba_start_seq_ctrl, 2);
 		tag += 2;
 	}
 

@@ -124,7 +124,7 @@ static struct sk_buff *rtllib_ADDBA(struct rtllib_device *ieee, u8 *Dst,
 }
 
 static struct sk_buff *rtllib_DELBA(struct rtllib_device *ieee, u8 *dst,
-				    struct ba_record *pBA,
+				    struct ba_record *pba,
 				    enum tr_select TxRxSelect, u16 ReasonCode)
 {
 	union delba_param_set DelbaParamSet;

@@ -140,7 +140,7 @@ static struct sk_buff *rtllib_DELBA(struct rtllib_device *ieee, u8 *dst,
 	memset(&DelbaParamSet, 0, 2);
 
 	DelbaParamSet.field.initiator = (TxRxSelect == TX_DIR) ? 1 : 0;
-	DelbaParamSet.field.tid	= pBA->ba_param_set.field.tid;
+	DelbaParamSet.field.tid	= pba->ba_param_set.field.tid;
 
 	skb = dev_alloc_skb(len + sizeof(struct rtllib_hdr_3addr));
 	if (!skb)

@@ -174,11 +174,11 @@ static struct sk_buff *rtllib_DELBA(struct rtllib_device *ieee, u8 *dst,
 }
 
 static void rtllib_send_ADDBAReq(struct rtllib_device *ieee, u8 *dst,
-				 struct ba_record *pBA)
+				 struct ba_record *pba)
 {
 	struct sk_buff *skb;
 
-	skb = rtllib_ADDBA(ieee, dst, pBA, 0, ACT_ADDBAREQ);
+	skb = rtllib_ADDBA(ieee, dst, pba, 0, ACT_ADDBAREQ);
 
 	if (skb)
 		softmac_mgmt_xmit(skb, ieee);

@@ -187,11 +187,11 @@ static void rtllib_send_ADDBAReq(struct rtllib_device *ieee, u8 *dst,
 }
 
 static void rtllib_send_ADDBARsp(struct rtllib_device *ieee, u8 *dst,
-				 struct ba_record *pBA, u16 StatusCode)
+				 struct ba_record *pba, u16 StatusCode)
 {
 	struct sk_buff *skb;
 
-	skb = rtllib_ADDBA(ieee, dst, pBA, StatusCode, ACT_ADDBARSP);
+	skb = rtllib_ADDBA(ieee, dst, pba, StatusCode, ACT_ADDBARSP);
 	if (skb)
 		softmac_mgmt_xmit(skb, ieee);
 	else

@@ -199,12 +199,12 @@ static void rtllib_send_ADDBARsp(struct rtllib_device *ieee, u8 *dst,
 }
 
 static void rtllib_send_DELBA(struct rtllib_device *ieee, u8 *dst,
-			      struct ba_record *pBA, enum tr_select TxRxSelect,
+			      struct ba_record *pba, enum tr_select TxRxSelect,
 			      u16 ReasonCode)
 {
 	struct sk_buff *skb;
 
-	skb = rtllib_DELBA(ieee, dst, pBA, TxRxSelect, ReasonCode);
+	skb = rtllib_DELBA(ieee, dst, pba, TxRxSelect, ReasonCode);
 	if (skb)
 		softmac_mgmt_xmit(skb, ieee);
 	else

@@ -216,7 +216,7 @@ int rtllib_rx_ADDBAReq(struct rtllib_device *ieee, struct sk_buff *skb)
 	struct rtllib_hdr_3addr *req = NULL;
 	u16 rc = 0;
 	u8 *dst = NULL, *pDialogToken = NULL, *tag = NULL;
-	struct ba_record *pBA = NULL;
+	struct ba_record *pba = NULL;
 	union ba_param_set *pBaParamSet = NULL;
 	u16 *pBaTimeoutVal = NULL;
 	union sequence_control *pBaStartSeqCtrl = NULL;

@@ -259,7 +259,7 @@ int rtllib_rx_ADDBAReq(struct rtllib_device *ieee, struct sk_buff *skb)
 		netdev_warn(ieee->dev, "%s(): can't get TS\n", __func__);
 		goto OnADDBAReq_Fail;
 	}
-	pBA = &pTS->rx_admitted_ba_record;
+	pba = &pTS->rx_admitted_ba_record;
 
 	if (pBaParamSet->field.ba_policy == BA_POLICY_DELAYED) {
 		rc = ADDBA_STATUS_INVALID_PARAM;

@@ -270,20 +270,20 @@ int rtllib_rx_ADDBAReq(struct rtllib_device *ieee, struct sk_buff *skb)
 
 	rtllib_FlushRxTsPendingPkts(ieee, pTS);
 
-	deactivate_ba_entry(ieee, pBA);
-	pBA->dialog_token = *pDialogToken;
-	pBA->ba_param_set = *pBaParamSet;
-	pBA->ba_timeout_value = *pBaTimeoutVal;
-	pBA->ba_start_seq_ctrl = *pBaStartSeqCtrl;
+	deactivate_ba_entry(ieee, pba);
+	pba->dialog_token = *pDialogToken;
+	pba->ba_param_set = *pBaParamSet;
+	pba->ba_timeout_value = *pBaTimeoutVal;
+	pba->ba_start_seq_ctrl = *pBaStartSeqCtrl;
 
 	if (ieee->GetHalfNmodeSupportByAPsHandler(ieee->dev) ||
 	   (ieee->ht_info->iot_action & HT_IOT_ACT_ALLOW_PEER_AGG_ONE_PKT))
-		pBA->ba_param_set.field.buffer_size = 1;
+		pba->ba_param_set.field.buffer_size = 1;
 	else
-		pBA->ba_param_set.field.buffer_size = 32;
+		pba->ba_param_set.field.buffer_size = 32;
 
-	activate_ba_entry(pBA, 0);
-	rtllib_send_ADDBARsp(ieee, dst, pBA, ADDBA_STATUS_SUCCESS);
+	activate_ba_entry(pba, 0);
+	rtllib_send_ADDBARsp(ieee, dst, pba, ADDBA_STATUS_SUCCESS);
 
 	return 0;
 

@@ -464,24 +464,24 @@ int rtllib_rx_DELBA(struct rtllib_device *ieee, struct sk_buff *skb)
 void ts_init_add_ba(struct rtllib_device *ieee, struct tx_ts_record *pTS,
 		 u8 Policy, u8	bOverwritePending)
 {
-	struct ba_record *pBA = &pTS->TxPendingBARecord;
+	struct ba_record *pba = &pTS->TxPendingBARecord;
 
-	if (pBA->b_valid && !bOverwritePending)
+	if (pba->b_valid && !bOverwritePending)
 		return;
 
-	deactivate_ba_entry(ieee, pBA);
+	deactivate_ba_entry(ieee, pba);
 
-	pBA->dialog_token++;
-	pBA->ba_param_set.field.amsdu_support = 0;
-	pBA->ba_param_set.field.ba_policy = Policy;
-	pBA->ba_param_set.field.tid = pTS->TsCommonInfo.TSpec.f.TSInfo.field.ucTSID;
-	pBA->ba_param_set.field.buffer_size = 32;
-	pBA->ba_timeout_value = 0;
-	pBA->ba_start_seq_ctrl.field.seq_num = (pTS->TxCurSeq + 3) % 4096;
+	pba->dialog_token++;
+	pba->ba_param_set.field.amsdu_support = 0;
+	pba->ba_param_set.field.ba_policy = Policy;
+	pba->ba_param_set.field.tid = pTS->TsCommonInfo.TSpec.f.TSInfo.field.ucTSID;
+	pba->ba_param_set.field.buffer_size = 32;
+	pba->ba_timeout_value = 0;
+	pba->ba_start_seq_ctrl.field.seq_num = (pTS->TxCurSeq + 3) % 4096;
 
-	activate_ba_entry(pBA, BA_SETUP_TIMEOUT);
+	activate_ba_entry(pba, BA_SETUP_TIMEOUT);
 
-	rtllib_send_ADDBAReq(ieee, pTS->TsCommonInfo.Addr, pBA);
+	rtllib_send_ADDBAReq(ieee, pTS->TsCommonInfo.Addr, pba);
 }
 
 void ts_init_del_ba(struct rtllib_device *ieee,
drivers/staging/rtl8192e/rtllib.h +1 -1
index 0f84ee1d418e..8f0e9dbe5b7a 100644
@@ -2016,7 +2016,7 @@ void ts_init_del_ba(struct rtllib_device *ieee,
 void ba_setup_time_out(struct timer_list *t);
 void tx_ba_inact_timeout(struct timer_list *t);
 void rx_ba_inact_timeout(struct timer_list *t);
-void ResetBaEntry(struct ba_record *pBA);
+void ResetBaEntry(struct ba_record *pba);
 bool GetTs(struct rtllib_device *ieee, struct ts_common_info **ppTS, u8 *Addr,
 	   u8 TID, enum tr_select TxRxSelect, bool bAddNewTs);
 void TSInitialize(struct rtllib_device *ieee);
Signature · 1 line
2.41.0
#1 lore raw
Quoted 8 lines · Tree Davies

Rename variable pBA to pba in order to Fix checkpatch
warning: Avoid CamelCase

Signed-off-by Tree Davies<[email protected]>
---
  drivers/staging/rtl8192e/rtl819x_BAProc.c | 106 +++++++++++-----------
  drivers/staging/rtl8192e/rtllib.h         |   2 +-
  2 files changed, 54 insertions(+), 54 deletions(-)

Hi Tree,
the p is typically for pointer. This is not wanted when you change the name. But ba is is in use....

Bye Philipp

author
#2 lore raw
Quoted 11 lines · Philipp Hortmann
Quoted 8 lines · Tree Davies

Rename variable pBA to pba in order to Fix checkpatch
warning: Avoid CamelCase

Signed-off-by Tree Davies<[email protected]>
---
  drivers/staging/rtl8192e/rtl819x_BAProc.c | 106 +++++++++++-----------
  drivers/staging/rtl8192e/rtllib.h         |   2 +-
  2 files changed, 54 insertions(+), 54 deletions(-)
Quoted 5 lines

Hi Tree,
the p is typically for pointer. This is not wanted when you change the name. But ba is is in use....

Bye Philipp

Thanks Philipp,

A few thoughts...
Looking at occurances of pBA, they all appear as local variable declarations of struct ba_record, mostly as function params.

I also see what you mentioned, as BA being already taken in rtl819x_BAProc.c:394 and line 292, but I don't 'think' that renaming them both to ba will result negatively(?).

Agreed, let's wait on Greg.

Cheers!
Tree

#3 lore raw
Quoted 24 lines · Tree Davies
Quoted 11 lines · Philipp Hortmann
Quoted 8 lines · Tree Davies

Rename variable pBA to pba in order to Fix checkpatch
warning: Avoid CamelCase

Signed-off-by Tree Davies<[email protected]>
---
   drivers/staging/rtl8192e/rtl819x_BAProc.c | 106 +++++++++++-----------
   drivers/staging/rtl8192e/rtllib.h         |   2 +-
   2 files changed, 54 insertions(+), 54 deletions(-)
Quoted 5 lines

Hi Tree,
the p is typically for pointer. This is not wanted when you change the name. But ba is is in use....

Bye Philipp

Thanks Philipp,

A few thoughts...
Looking at occurances of pBA, they all appear as local variable declarations of struct ba_record, mostly as function params.

Quoted 9 lines

I also see what you mentioned, as BA being already taken in rtl819x_BAProc.c:394 and line 292, but I don't 'think' that renaming them both to ba will result negatively(?).

Agreed, let's wait on Greg.

Cheers!
Tree

Hi Tree,

it is not so much about the compiler or if it works this way. When I read a program I often use Ctrl+Shift+F what shows me all variables in one folder (driver). I expect that I then always see one variable for one use and do not have to do a one by one decision if this belongs to the one content or to the other.

Please consider "readability" is important for kernel code.

Thanks for your support.

Bye Philipp

#4 lore raw
In reply to Tree Davies
Quoted 16 lines · Tree Davies
Quoted 15 lines · Philipp Hortmann
Quoted 8 lines · Tree Davies

Rename variable pBA to pba in order to Fix checkpatch
warning: Avoid CamelCase

Signed-off-by Tree Davies<[email protected]>
---
  drivers/staging/rtl8192e/rtl819x_BAProc.c | 106 +++++++++++-----------
  drivers/staging/rtl8192e/rtllib.h         |   2 +-
  2 files changed, 54 insertions(+), 54 deletions(-)

Hi Tree,
the p is typically for pointer. This is not wanted when you change the name. But ba is is in use....

Quoted 7 lines
Quoted 1 line

Bye Philipp

Thanks Philipp,

A few thoughts...
Looking at occurances of pBA, they all appear as local variable declarations of struct ba_record, mostly as function params.

I'm reading my inbox in the wrong order so I already sent an email with the same advice that Philipp sent. Philipp is correct. "p" is not allowed.

Quoted 4 lines

I also see what you mentioned, as BA being already taken in rtl819x_BAProc.c:394 and line 292, but I don't 'think' that renaming them both to ba will result negatively(?).

Thanks for noticing this. Figure out a solution.

Quoted 2 lines

Agreed, let's wait on Greg.

Heh. Greg is not going to fix these minor issues and neither am I. We trust you to find a solution. You can do it! I believe in you!

regards,
dan carpenter

#5 lore raw
In reply to Tree Davies
Quoted 14 lines · Tree Davies

Rename variable pBA to pba in order to Fix checkpatch
warning: Avoid CamelCase

Signed-off-by Tree Davies <[email protected]>
---
 drivers/staging/rtl8192e/rtl819x_BAProc.c | 106 +++++++++++-----------
 drivers/staging/rtl8192e/rtllib.h         |   2 +-
 2 files changed, 54 insertions(+), 54 deletions(-)
drivers/staging/rtl8192e/rtl819x_BAProc.c +0 -0
index 6b5da38353ee..43ee1bd4a6ed 100644

drivers/staging/rtl8192e/rtl819x_BAProc.c line 13

Quoted 5 lines
drivers/staging/rtl8192e/rtl819x_BAProc.c +1 -1
@@ -10,17 +10,17 @@
 #include "rtllib.h"
 #include "rtl819x_BA.h"
 
-static void activate_ba_entry(struct ba_record *pBA, u16 Time)
+static void activate_ba_entry(struct ba_record *pba, u16 Time)

Here the "p" stands for "pointer". This style of naming is not allowed in the Linux kernel. Just use "ba" as a name. This happens in a couple other patches as well so fix those too.

regards,
dan carpenter