[PATCH 09/12] Staging: rtl8192e: Rename variable pBA

[PATCH 09/12] Staging: rtl8192e: Rename variable pBA

[Tree Davies]

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

Signed-off-by: Tree Davies <tdavies@darkphysics.net>
---
 drivers/staging/rtl8192e/rtl819x_BAProc.c | 106 +++++++++++-----------
 drivers/staging/rtl8192e/rtllib.h         |   2 +-
 2 files changed, 54 insertions(+), 54 deletions(-)

diff --git a/drivers/staging/rtl8192e/rtl819x_BAProc.c b/drivers/staging/rtl8192e/rtl819x_BAProc.c
index 6b5da38353ee..43ee1bd4a6ed 100644
--- a/drivers/staging/rtl8192e/rtl819x_BAProc.c
+++ b/drivers/staging/rtl8192e/rtl819x_BAProc.c
@@ -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,
diff --git a/drivers/staging/rtl8192e/rtllib.h b/drivers/staging/rtl8192e/rtllib.h
index 0f84ee1d418e..8f0e9dbe5b7a 100644
--- a/drivers/staging/rtl8192e/rtllib.h
+++ b/drivers/staging/rtl8192e/rtllib.h
@@ -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);
-- 
2.41.0

Re: [PATCH 09/12] Staging: rtl8192e: Rename variable pBA

[Philipp Hortmann]

On 7/13/23 01:35, Tree Davies wrote:
> Rename variable pBA to pba in order to Fix checkpatch
> warning: Avoid CamelCase
> 
> Signed-off-by: Tree Davies<tdavies@darkphysics.net>
> ---
>   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

Re: [PATCH 09/12] Staging: rtl8192e: Rename variable pBA

[Tree Davies]

On Thu, Jul 13, 2023 at 11:54:40PM +0200, Philipp Hortmann wrote:
> On 7/13/23 01:35, Tree Davies wrote:
> > Rename variable pBA to pba in order to Fix checkpatch
> > warning: Avoid CamelCase
> > 
> > Signed-off-by: Tree Davies<tdavies@darkphysics.net>
> > ---
> >   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

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

Re: [PATCH 09/12] Staging: rtl8192e: Rename variable pBA

[Philipp Hortmann]

On 7/14/23 04:49, Tree Davies wrote:
> On Thu, Jul 13, 2023 at 11:54:40PM +0200, Philipp Hortmann wrote:
>> On 7/13/23 01:35, Tree Davies wrote:
>>> Rename variable pBA to pba in order to Fix checkpatch
>>> warning: Avoid CamelCase
>>>
>>> Signed-off-by: Tree Davies<tdavies@darkphysics.net>
>>> ---
>>>    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
> 
> 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
> 

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

Re: [PATCH 09/12] Staging: rtl8192e: Rename variable pBA

[Dan Carpenter]

On Wed, Jul 12, 2023 at 04:35:13PM -0700, Tree Davies wrote:
> Rename variable pBA to pba in order to Fix checkpatch
> warning: Avoid CamelCase
> 
> Signed-off-by: Tree Davies <tdavies@darkphysics.net>
> ---
>  drivers/staging/rtl8192e/rtl819x_BAProc.c | 106 +++++++++++-----------
>  drivers/staging/rtl8192e/rtllib.h         |   2 +-
>  2 files changed, 54 insertions(+), 54 deletions(-)
> 
> diff --git a/drivers/staging/rtl8192e/rtl819x_BAProc.c b/drivers/staging/rtl8192e/rtl819x_BAProc.c
> index 6b5da38353ee..43ee1bd4a6ed 100644
> --- a/drivers/staging/rtl8192e/rtl819x_BAProc.c
> +++ b/drivers/staging/rtl8192e/rtl819x_BAProc.c
> @@ -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

Re: [PATCH 09/12] Staging: rtl8192e: Rename variable pBA

[Dan Carpenter]

On Thu, Jul 13, 2023 at 07:49:39PM -0700, Tree Davies wrote:
> On Thu, Jul 13, 2023 at 11:54:40PM +0200, Philipp Hortmann wrote:
> > On 7/13/23 01:35, Tree Davies wrote:
> > > Rename variable pBA to pba in order to Fix checkpatch
> > > warning: Avoid CamelCase
> > > 
> > > Signed-off-by: Tree Davies<tdavies@darkphysics.net>
> > > ---
> > >   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
> 
> 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.

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

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