Skip to contentSkip to conversation

staging: Drop unnecessary code

PATCH Superseded Amitoj Kaur Chawla 3 participants 9 messages Oct 31, 2015 – Nov 4, 2015 Staging

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

cover staging: Drop unnecessary code

This patchset removes wrapper function, unnecessary variable and function to clean up code.

Amitoj Kaur Chawla (4):
staging: rtl8712: rtl871x_sta_mgt: Remove wrapper function
staging: rtl8712: rtl871x_mlme: Remove wrapper function
staging: rtl8192e: Remove unnecessary variable
staging: rtl8192u: Remove unnecessary function

 drivers/staging/rtl8192e/rtl8192e/r8192E_phy.c |  5 ++---
 drivers/staging/rtl8192u/r819xU_phy.c          | 25 +++++--------------------
 drivers/staging/rtl8712/rtl871x_mlme.c         |  8 +-------
 drivers/staging/rtl8712/rtl871x_sta_mgt.c      |  9 ++-------
 4 files changed, 10 insertions(+), 37 deletions(-)
Signature · 1 line
1.9.1
#1 lore raw

PATCH 1/4 staging: rtl8712: rtl871x_sta_mgt: Remove wrapper function

Remove wrapper function that can be replaced by a single line of code.

---
 drivers/staging/rtl8712/rtl871x_sta_mgt.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)
drivers/staging/rtl8712/rtl871x_sta_mgt.c +2 -7
index 6ae8cdc..a379b84 100644
@@ -89,16 +89,11 @@ static void mfree_all_stainfo(struct sta_priv *pstapriv)
 	spin_unlock_irqrestore(&pstapriv->sta_hash_lock, irqL);
 }
 
-
-static void mfree_sta_priv_lock(struct	sta_priv *pstapriv)
-{
-	 mfree_all_stainfo(pstapriv); /* be done before free sta_hash_lock */
-}
-
 u32 _r8712_free_sta_priv(struct sta_priv *pstapriv)
 {
 	if (pstapriv) {
-		mfree_sta_priv_lock(pstapriv);
+		/* be done before free sta_hash_lock */
+		mfree_all_stainfo(pstapriv); 
 		kfree(pstapriv->pallocated_stainfo_buf);
 	}
 	return _SUCCESS;
Signature · 1 line
1.9.1
maintainer
#2 lore raw

Re: [Outreachy kernel] [PATCH 1/4] staging: rtl8712: rtl871x_sta_mgt: Remove wrapper function

Quoted 17 lines · Amitoj Kaur Chawla

Remove wrapper function that can be replaced by a single line of code.

---
 drivers/staging/rtl8712/rtl871x_sta_mgt.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)
drivers/staging/rtl8712/rtl871x_sta_mgt.c +0 -2
index 6ae8cdc..a379b84 100644
@@ -89,16 +89,11 @@ static void mfree_all_stainfo(struct sta_priv *pstapriv)
 	spin_unlock_irqrestore(&pstapriv->sta_hash_lock, irqL);
 }
 
-
-static void mfree_sta_priv_lock(struct	sta_priv *pstapriv)

drivers/staging/rtl8712/rtl871x_sta_mgt.c static void mfree_all_stainfo(struct sta_priv *pstapriv) · line 96

Quoted 10 lines
drivers/staging/rtl8712/rtl871x_sta_mgt.c +2 -5
-{
-	 mfree_all_stainfo(pstapriv); /* be done before free sta_hash_lock */
-}
-
 u32 _r8712_free_sta_priv(struct sta_priv *pstapriv)
 {
 	if (pstapriv) {
-		mfree_sta_priv_lock(pstapriv);
+		/* be done before free sta_hash_lock */
+		mfree_all_stainfo(pstapriv); 

You add trailing whitespace :(

Always check your patches with checkpatch before sending them out.

thanks,

greg k-h

#3 lore raw

PATCH 2/4 staging: rtl8712: rtl871x_mlme: Remove wrapper function

In reply to Amitoj Kaur Chawla

Remove wrapper function free_network_nolock() that can be replaced by a single line of code.

This patch renames _free_network_nolock() function to free_network_nolock().

---
 drivers/staging/rtl8712/rtl871x_mlme.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)
drivers/staging/rtl8712/rtl871x_mlme.c +1 -7
index fbcb248..8972c15 100644
@@ -123,7 +123,7 @@ static void _free_network(struct mlme_priv *pmlmepriv,
 	spin_unlock_irqrestore(&free_queue->lock, irqL);
 }
 
-static void _free_network_nolock(struct mlme_priv *pmlmepriv,
+static void free_network_nolock(struct mlme_priv *pmlmepriv,
 			  struct wlan_network *pnetwork)
 {
 	struct  __queue *free_queue = &pmlmepriv->free_bss_pool;
@@ -233,12 +233,6 @@ static struct	wlan_network *alloc_network(struct mlme_priv *pmlmepriv)
 	return _r8712_alloc_network(pmlmepriv);
 }
 
-static void free_network_nolock(struct mlme_priv *pmlmepriv,
-			 struct wlan_network *pnetwork)
-{
-	_free_network_nolock(pmlmepriv, pnetwork);
-}
-
 void r8712_free_network_queue(struct _adapter *dev)
 {
 	_free_network_queue(dev);
Signature · 1 line
1.9.1
#4 lore raw

PATCH 3/4 staging: rtl8192e: Remove unnecessary variable

In reply to Amitoj Kaur Chawla

This patch removes unnecessary variable by using a single line of code instead.

---
 drivers/staging/rtl8192e/rtl8192e/r8192E_phy.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
drivers/staging/rtl8192e/rtl8192e/r8192E_phy.c +2 -3
index 0b407fe..5e3bbe5 100644
@@ -90,13 +90,12 @@ void rtl92e_set_bb_reg(struct net_device *dev, u32 dwRegAddr, u32 dwBitMask,
 
 u32 rtl92e_get_bb_reg(struct net_device *dev, u32 dwRegAddr, u32 dwBitMask)
 {
-	u32 Ret = 0, OriginalValue, BitShift;
+	u32 OriginalValue, BitShift;
 
 	OriginalValue = rtl92e_readl(dev, dwRegAddr);
 	BitShift = _rtl92e_calculate_bit_shift(dwBitMask);
-	Ret = (OriginalValue & dwBitMask) >> BitShift;
 
-	return Ret;
+	return (OriginalValue & dwBitMask) >> BitShift;
 }
 
 static u32 _rtl92e_phy_rf_read(struct net_device *dev,
Signature · 1 line
1.9.1
#5 lore raw

PATCH 4/4 staging: rtl8192u: Remove unnecessary function

In reply to Amitoj Kaur Chawla

This patch solves two problems.
The function rtl8192_CalculateBitShift() had an unnecessary variable i that could be removed by using a single line of code.

After this change, rtl8192_CalculateBitShift() becomes a wrapper function, so the rtl8192_CalculateBitShift() function has been removed completely to replace it with a single line of code.

---
 drivers/staging/rtl8192u/r819xU_phy.c | 25 +++++--------------------
 1 file changed, 5 insertions(+), 20 deletions(-)
drivers/staging/rtl8192u/r819xU_phy.c +5 -20
index e5dbaca..5ee4c34 100644
@@ -38,21 +38,6 @@ static u32 RF_CHANNEL_TABLE_ZEBRA[] = {
 #define rtl819XAGCTAB_Array Rtl8192UsbAGCTAB_Array
 
 /******************************************************************************
- * function: This function reads BB parameters from header file we generate,
- *           and does register read/write
- * input:    u32	bitmask  //taget bit pos in the addr to be modified
- * output:   none
- * return:   u32	return the shift bit position of the mask
- ******************************************************************************/
-static u32 rtl8192_CalculateBitShift(u32 bitmask)
-{
-	u32 i;
-
-	i = ffs(bitmask) - 1;
-	return i;
-}
-
-/******************************************************************************
  * function:  This function checks different RF type to execute legal judgement.
  *            If RF Path is illegal, we will return false.
  * input:     net_device	 *dev
@@ -94,7 +79,7 @@ void rtl8192_setBBreg(struct net_device *dev, u32 reg_addr, u32 bitmask,
 
 	if (bitmask != bMaskDWord) {
 		read_nic_dword(dev, reg_addr, &reg);
-		bitshift = rtl8192_CalculateBitShift(bitmask);
+		bitshift = ffs(bitmask) - 1;
 		reg &= ~bitmask;
 		reg |= data << bitshift;
 		write_nic_dword(dev, reg_addr, reg);
@@ -117,7 +102,7 @@ u32 rtl8192_QueryBBReg(struct net_device *dev, u32 reg_addr, u32 bitmask)
 	u32 reg, bitshift;
 
 	read_nic_dword(dev, reg_addr, &reg);
-	bitshift = rtl8192_CalculateBitShift(bitmask);
+	bitshift = ffs(bitmask) - 1;
 
 	return (reg & bitmask) >> bitshift;
 }
@@ -306,7 +291,7 @@ void rtl8192_phy_SetRFReg(struct net_device *dev, RF90_RADIO_PATH_E eRFPath,
 		if (bitmask != bMask12Bits) {
 			/* RF data is 12 bits only */
 			reg = phy_FwRFSerialRead(dev, eRFPath, reg_addr);
-			bitshift =  rtl8192_CalculateBitShift(bitmask);
+			bitshift =  ffs(bitmask) - 1;
 			reg &= ~bitmask;
 			reg |= data << bitshift;
 
@@ -321,7 +306,7 @@ void rtl8192_phy_SetRFReg(struct net_device *dev, RF90_RADIO_PATH_E eRFPath,
 		if (bitmask != bMask12Bits) {
 			/* RF data is 12 bits only */
 			reg = rtl8192_phy_RFSerialRead(dev, eRFPath, reg_addr);
-			bitshift =  rtl8192_CalculateBitShift(bitmask);
+			bitshift =  ffs(bitmask) - 1;
 			reg &= ~bitmask;
 			reg |= data << bitshift;
 
@@ -356,7 +341,7 @@ u32 rtl8192_phy_QueryRFReg(struct net_device *dev, RF90_RADIO_PATH_E eRFPath,
 	} else {
 		reg = rtl8192_phy_RFSerialRead(dev, eRFPath, reg_addr);
 	}
-	bitshift =  rtl8192_CalculateBitShift(bitmask);
+	bitshift =  ffs(bitmask) - 1;
 	reg = (reg & bitmask) >> bitshift;
 	return reg;
 
Signature · 1 line
1.9.1
#6 lore raw

Re: [Outreachy kernel] [PATCH 4/4] staging: rtl8192u: Remove unnecessary function

Quoted 40 lines · Amitoj Kaur Chawla

This patch solves two problems.
The function rtl8192_CalculateBitShift() had an unnecessary variable i that could be removed by using a single line of code.

After this change, rtl8192_CalculateBitShift() becomes a wrapper function, so the rtl8192_CalculateBitShift() function has been removed completely to replace it with a single line of code.

---
 drivers/staging/rtl8192u/r819xU_phy.c | 25 +++++--------------------
 1 file changed, 5 insertions(+), 20 deletions(-)
drivers/staging/rtl8192u/r819xU_phy.c +0 -15
index e5dbaca..5ee4c34 100644
@@ -38,21 +38,6 @@ static u32 RF_CHANNEL_TABLE_ZEBRA[] = {
 #define rtl819XAGCTAB_Array Rtl8192UsbAGCTAB_Array
 
 /******************************************************************************
- * function: This function reads BB parameters from header file we generate,
- *           and does register read/write
- * input:    u32	bitmask  //taget bit pos in the addr to be modified
- * output:   none
- * return:   u32	return the shift bit position of the mask
- ******************************************************************************/
-static u32 rtl8192_CalculateBitShift(u32 bitmask)
-{
-	u32 i;
-
-	i = ffs(bitmask) - 1;
-	return i;
-}
-
-/******************************************************************************
  * function:  This function checks different RF type to execute legal judgement.
  *            If RF Path is illegal, we will return false.
  * input:     net_device	 *dev

drivers/staging/rtl8192u/r819xU_phy.c void rtl8192_setBBreg(struct net_device *dev, u32 reg_addr, u32 bitmask, · line 82

Quoted 5 lines
drivers/staging/rtl8192u/r819xU_phy.c +1 -1
@@ -94,7 +79,7 @@ void rtl8192_setBBreg(struct net_device *dev, u32 reg_addr, u32 bitmask,
 
 	if (bitmask != bMaskDWord) {
 		read_nic_dword(dev, reg_addr, &reg);
-		bitshift = rtl8192_CalculateBitShift(bitmask);
+		bitshift = ffs(bitmask) - 1;

I'm not sure that the change is a good idea. The names ffs is not very understandable. Is it a standard kernel function? What does it do?

julia

Quoted 49 lines
drivers/staging/rtl8192u/r819xU_phy.c +4 -4
 		reg &= ~bitmask;
 		reg |= data << bitshift;
 		write_nic_dword(dev, reg_addr, reg);
@@ -117,7 +102,7 @@ u32 rtl8192_QueryBBReg(struct net_device *dev, u32 reg_addr, u32 bitmask)
 	u32 reg, bitshift;
 
 	read_nic_dword(dev, reg_addr, &reg);
-	bitshift = rtl8192_CalculateBitShift(bitmask);
+	bitshift = ffs(bitmask) - 1;
 
 	return (reg & bitmask) >> bitshift;
 }
@@ -306,7 +291,7 @@ void rtl8192_phy_SetRFReg(struct net_device *dev, RF90_RADIO_PATH_E eRFPath,
 		if (bitmask != bMask12Bits) {
 			/* RF data is 12 bits only */
 			reg = phy_FwRFSerialRead(dev, eRFPath, reg_addr);
-			bitshift =  rtl8192_CalculateBitShift(bitmask);
+			bitshift =  ffs(bitmask) - 1;
 			reg &= ~bitmask;
 			reg |= data << bitshift;
 
@@ -321,7 +306,7 @@ void rtl8192_phy_SetRFReg(struct net_device *dev, RF90_RADIO_PATH_E eRFPath,
 		if (bitmask != bMask12Bits) {
 			/* RF data is 12 bits only */
 			reg = rtl8192_phy_RFSerialRead(dev, eRFPath, reg_addr);
-			bitshift =  rtl8192_CalculateBitShift(bitmask);
+			bitshift =  ffs(bitmask) - 1;
 			reg &= ~bitmask;
 			reg |= data << bitshift;
 
@@ -356,7 +341,7 @@ u32 rtl8192_phy_QueryRFReg(struct net_device *dev, RF90_RADIO_PATH_E eRFPath,
 	} else {
 		reg = rtl8192_phy_RFSerialRead(dev, eRFPath, reg_addr);
 	}
-	bitshift =  rtl8192_CalculateBitShift(bitmask);
+	bitshift =  ffs(bitmask) - 1;
 	reg = (reg & bitmask) >> bitshift;
 	return reg;
 
Signature · 8 lines
1.9.1

-- 
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/57ba37d09c11afef9ae42a4d251771796479cbb4.1446302505.git.amitoj1606%40gmail.com.
For more options, visit https://groups.google.com/d/optout.
#7 lore raw

Re: [Outreachy kernel] [PATCH 4/4] staging: rtl8192u: Remove unnecessary function

Quoted 10 lines · Julia Lawall
Quoted 8 lines · Amitoj Kaur Chawla

This patch solves two problems.
The function rtl8192_CalculateBitShift() had an unnecessary variable i that could be removed by using a single line of code.

After this change, rtl8192_CalculateBitShift() becomes a wrapper function, so the rtl8192_CalculateBitShift() function has been removed completely to replace it with a single line of code.

Quoted 6 lines

Review

Quoted 6 lines
---
 drivers/staging/rtl8192u/r819xU_phy.c | 25 +++++--------------------
 1 file changed, 5 insertions(+), 20 deletions(-)
(quoted diff) +0 -0

b/drivers/staging/rtl8192u/r819xU_phy.c

Quoted 7 lines

drivers/staging/rtl8192u/r819xU_phy.c static u32 RF_CHANNEL_TABLE_ZEBRA[] = { · line 40

Quoted 7 lines

index e5dbaca..5ee4c34 100644

drivers/staging/rtl8192u/r819xU_phy.c +0 -0
@@ -38,21 +38,6 @@ static u32 RF_CHANNEL_TABLE_ZEBRA[] = {
 #define rtl819XAGCTAB_Array Rtl8192UsbAGCTAB_Array
 
 

/******************************************************************************

Quoted 1 line

drivers/staging/rtl8192u/r819xU_phy.c static u32 RF_CHANNEL_TABLE_ZEBRA[] = { · line 41

Quoted 1 line
drivers/staging/rtl8192u/r819xU_phy.c +0 -1
- * function: This function reads BB parameters from header file we

generate,

Quoted 5 lines

drivers/staging/rtl8192u/r819xU_phy.c static u32 RF_CHANNEL_TABLE_ZEBRA[] = { · line 50

Quoted 5 lines
drivers/staging/rtl8192u/r819xU_phy.c +0 -5
- *           and does register read/write
- * input:    u32     bitmask  //taget bit pos in the addr to be modified
- * output:   none
- * return:   u32     return the shift bit position of the mask
-

******************************************************************************/

Quoted 9 lines

drivers/staging/rtl8192u/r819xU_phy.c static u32 RF_CHANNEL_TABLE_ZEBRA[] = {

Quoted 9 lines
drivers/staging/rtl8192u/r819xU_phy.c +0 -8
-static u32 rtl8192_CalculateBitShift(u32 bitmask)
-{
-     u32 i;
-
-     i = ffs(bitmask) - 1;
-     return i;
-}
-
 

-/******************************************************************************

Quoted 1 line

drivers/staging/rtl8192u/r819xU_phy.c static u32 RF_CHANNEL_TABLE_ZEBRA[] = { · line 42

Quoted 1 line
drivers/staging/rtl8192u/r819xU_phy.c +0 -0
  * function:  This function checks different RF type to execute legal

judgement.

Quoted 3 lines

drivers/staging/rtl8192u/r819xU_phy.c void rtl8192_setBBreg(struct net_device *dev, u32

Quoted 3 lines
drivers/staging/rtl8192u/r819xU_phy.c +0 -0
  *            If RF Path is illegal, we will return false.
  * input:     net_device      *dev
@@ -94,7 +79,7 @@ void rtl8192_setBBreg(struct net_device *dev, u32

reg_addr, u32 bitmask,

Quoted 10 lines

drivers/staging/rtl8192u/r819xU_phy.c void rtl8192_setBBreg(struct net_device *dev, u32 reg_addr, u32 bitmask, · line 82

Quoted 5 lines
drivers/staging/rtl8192u/r819xU_phy.c +1 -1
      if (bitmask != bMaskDWord) {
              read_nic_dword(dev, reg_addr, &reg);
-             bitshift = rtl8192_CalculateBitShift(bitmask);
+             bitshift = ffs(bitmask) - 1;

I'm not sure that the change is a good idea. The names ffs is not very understandable. Is it a standard kernel function? What does it do?

julia

As far as I read up about it, it returns the least significant bit of bitmask. I read up about it here.
<http://man7.org/linux/man-pages/man3/ffs.3.html>

Confused as to why the change isn't a good idea.

Signature · 1 line
Amitoj
#8 lore raw

Re: [Outreachy kernel] [PATCH 4/4] staging: rtl8192u: Remove unnecessary function

Quoted 58 lines · Amitoj Kaur Chawla
Quoted 10 lines · Julia Lawall
Quoted 8 lines · Amitoj Kaur Chawla

This patch solves two problems.
The function rtl8192_CalculateBitShift() had an unnecessary variable i that could be removed by using a single line of code.

After this change, rtl8192_CalculateBitShift() becomes a wrapper function, so the rtl8192_CalculateBitShift() function has been removed completely to replace it with a single line of code.

Quoted 6 lines

Review

Quoted 6 lines

---
 drivers/staging/rtl8192u/r819xU_phy.c | 25 +++++--------------------

 1 file changed, 5 insertions(+), 20 deletions(-)
(quoted diff) +0 -0

b/drivers/staging/rtl8192u/r819xU_phy.c

Quoted 7 lines

drivers/staging/rtl8192u/r819xU_phy.c static u32 RF_CHANNEL_TABLE_ZEBRA[] = { · line 40

Quoted 7 lines

index e5dbaca..5ee4c34 100644

drivers/staging/rtl8192u/r819xU_phy.c +0 -0
@@ -38,21 +38,6 @@ static u32 RF_CHANNEL_TABLE_ZEBRA[] = {
  #define rtl819XAGCTAB_Array Rtl8192UsbAGCTAB_Array
 
  /*************************************************************************

*****

Quoted 1 line

drivers/staging/rtl8192u/r819xU_phy.c static u32 RF_CHANNEL_TABLE_ZEBRA[] = { · line 41

Quoted 1 line
drivers/staging/rtl8192u/r819xU_phy.c +0 -1
- * function: This function reads BB parameters from header file we

generate,

Quoted 5 lines

drivers/staging/rtl8192u/r819xU_phy.c static u32 RF_CHANNEL_TABLE_ZEBRA[] = {

Quoted 5 lines
drivers/staging/rtl8192u/r819xU_phy.c +0 -5
- *           and does register read/write
- * input:    u32     bitmask  //taget bit pos in the addr to be modified
- * output:   none
- * return:   u32     return the shift bit position of the mask
-***************************************************************************

***/

Quoted 9 lines

drivers/staging/rtl8192u/r819xU_phy.c static u32 RF_CHANNEL_TABLE_ZEBRA[] = {

Quoted 9 lines
drivers/staging/rtl8192u/r819xU_phy.c +0 -9
-static u32 rtl8192_CalculateBitShift(u32 bitmask)
-{
-     u32 i;
-
-     i = ffs(bitmask) - 1;
-     return i;
-}
-
-/*************************************************************************

*****

Quoted 1 line

drivers/staging/rtl8192u/r819xU_phy.c static u32 RF_CHANNEL_TABLE_ZEBRA[] = { · line 41

Quoted 1 line
drivers/staging/rtl8192u/r819xU_phy.c +0 -0
   * function:  This function checks different RF type to execute legal

judgement.

Quoted 3 lines

drivers/staging/rtl8192u/r819xU_phy.c void rtl8192_setBBreg(struct net_device *dev, u32

Quoted 3 lines
drivers/staging/rtl8192u/r819xU_phy.c +0 -0
   *            If RF Path is illegal, we will return false.
   * input:     net_device      *dev
@@ -94,7 +79,7 @@ void rtl8192_setBBreg(struct net_device *dev, u32

reg_addr, u32 bitmask,

Quoted 6 lines
Quoted 5 lines
drivers/staging/rtl8192u/r819xU_phy.c +1 -1
       if (bitmask != bMaskDWord) {
               read_nic_dword(dev, reg_addr, &reg);
-             bitshift = rtl8192_CalculateBitShift(bitmask);
+             bitshift = ffs(bitmask) - 1;
Quoted 8 lines
Quoted 4 lines

I'm not sure that the change is a good idea.  The names ffs is not very understandable.  Is it a standard kernel function?  What does it do?

julia

As far as I read up about it, it returns the least significant bit of bitmask. I read up about it here.
Confused as to why the change isn't a good idea.

OK, since it is a standard function, it should be OK.

julia