[PATCH 0/3] staging: rtl8192u: Refactor u4bAcParam construction

[PATCH 0/3] staging: rtl8192u: Refactor u4bAcParam construction

[Tuomo Rinne]

This is a resubmission of patch series. 

This submission fixes the warnings reported by kbuild robot by passing
in u4bAcParam to cpu_to_le32s using pointer rather than value.

This submission also fixes erroneous line deletion reported by reviewer.

I've also amended the commit message of the last patch of the series for 
better explanation.


Tuomo Rinne (3):
  staging: rtl8192u: Remove unnecessary scope
  staging: rtl8192u: Improve code readability
  staging: rtl8192u: Convert u4bAcParam to little-endian

 drivers/staging/rtl8192u/r8192U_dm.c | 77 ++++++++++++++++++++----------------
 1 file changed, 43 insertions(+), 34 deletions(-)

-- 
2.1.4

[PATCH 1/3] staging: rtl8192u: Remove unnecessary scope

[Tuomo Rinne]

Remove scope unnecessary scope that is already enforced by the if
statements scope.

Signed-off-by: Tuomo Rinne <tuomo.rinne@gmail.com>
---
 drivers/staging/rtl8192u/r8192U_dm.c | 66 +++++++++++++++++-------------------
 1 file changed, 32 insertions(+), 34 deletions(-)

diff --git a/drivers/staging/rtl8192u/r8192U_dm.c b/drivers/staging/rtl8192u/r8192U_dm.c
index 975f707..94d898b 100644
--- a/drivers/staging/rtl8192u/r8192U_dm.c
+++ b/drivers/staging/rtl8192u/r8192U_dm.c
@@ -2300,43 +2300,41 @@ static void dm_check_edca_turbo(
 		 * Restore original EDCA according to the declaration of AP.
 		 */
 		if (priv->bcurrent_turbo_EDCA) {
+			u8		u1bAIFS;
+			u32		u4bAcParam;
+			struct ieee80211_qos_parameters *qos_parameters = &priv->ieee80211->current_network.qos_data.parameters;
+			u8 mode = priv->ieee80211->mode;
+
+			/*  For Each time updating EDCA parameter, reset EDCA turbo mode status. */
+			dm_init_edca_turbo(dev);
+			u1bAIFS = qos_parameters->aifs[0] * ((mode&(IEEE_G|IEEE_N_24G)) ? 9 : 20) + aSifsTime;
+			u4bAcParam = (((le16_to_cpu(qos_parameters->tx_op_limit[0])) << AC_PARAM_TXOP_LIMIT_OFFSET)|
+				((le16_to_cpu(qos_parameters->cw_max[0])) << AC_PARAM_ECW_MAX_OFFSET)|
+				((le16_to_cpu(qos_parameters->cw_min[0])) << AC_PARAM_ECW_MIN_OFFSET)|
+				((u32)u1bAIFS << AC_PARAM_AIFS_OFFSET));
+			/*write_nic_dword(dev, WDCAPARA_ADD[i], u4bAcParam);*/
+			write_nic_dword(dev, EDCAPARA_BE,  u4bAcParam);
+
+			/*
+			 * Check ACM bit.
+			 * If it is set, immediately set ACM control bit to downgrading AC for passing WMM testplan. Annie, 2005-12-13.
+			 */
 			{
-				u8		u1bAIFS;
-				u32		u4bAcParam;
-				struct ieee80211_qos_parameters *qos_parameters = &priv->ieee80211->current_network.qos_data.parameters;
-				u8 mode = priv->ieee80211->mode;
-
-				/*  For Each time updating EDCA parameter, reset EDCA turbo mode status. */
-				dm_init_edca_turbo(dev);
-				u1bAIFS = qos_parameters->aifs[0] * ((mode&(IEEE_G|IEEE_N_24G)) ? 9 : 20) + aSifsTime;
-				u4bAcParam = (((le16_to_cpu(qos_parameters->tx_op_limit[0])) << AC_PARAM_TXOP_LIMIT_OFFSET)|
-					((le16_to_cpu(qos_parameters->cw_max[0])) << AC_PARAM_ECW_MAX_OFFSET)|
-					((le16_to_cpu(qos_parameters->cw_min[0])) << AC_PARAM_ECW_MIN_OFFSET)|
-					((u32)u1bAIFS << AC_PARAM_AIFS_OFFSET));
-				/*write_nic_dword(dev, WDCAPARA_ADD[i], u4bAcParam);*/
-				write_nic_dword(dev, EDCAPARA_BE,  u4bAcParam);
-
-				/*
-				 * Check ACM bit.
-				 * If it is set, immediately set ACM control bit to downgrading AC for passing WMM testplan. Annie, 2005-12-13.
-				 */
-				{
-					/*  TODO:  Modified this part and try to set acm control in only 1 IO processing!! */
-
-					PACI_AIFSN	pAciAifsn = (PACI_AIFSN)&(qos_parameters->aifs[0]);
-					u8		AcmCtrl;
-
-					read_nic_byte(dev, AcmHwCtrl, &AcmCtrl);
-
-					if (pAciAifsn->f.ACM) { /*  ACM bit is 1. */
-						AcmCtrl |= AcmHw_BeqEn;
-					} else {	/* ACM bit is 0. */
-						AcmCtrl &= (~AcmHw_BeqEn);
-					}
+				/*  TODO:  Modified this part and try to set acm control in only 1 IO processing!! */
 
-					RT_TRACE(COMP_QOS, "SetHwReg8190pci(): [HW_VAR_ACM_CTRL] Write 0x%X\n", AcmCtrl);
-					write_nic_byte(dev, AcmHwCtrl, AcmCtrl);
+				PACI_AIFSN	pAciAifsn = (PACI_AIFSN)&(qos_parameters->aifs[0]);
+				u8		AcmCtrl;
+
+				read_nic_byte(dev, AcmHwCtrl, &AcmCtrl);
+
+				if (pAciAifsn->f.ACM) { /*  ACM bit is 1. */
+					AcmCtrl |= AcmHw_BeqEn;
+				} else {	/* ACM bit is 0. */
+					AcmCtrl &= (~AcmHw_BeqEn);
 				}
+
+				RT_TRACE(COMP_QOS, "SetHwReg8190pci(): [HW_VAR_ACM_CTRL] Write 0x%X\n", AcmCtrl);
+				write_nic_byte(dev, AcmHwCtrl, AcmCtrl);
 			}
 			priv->bcurrent_turbo_EDCA = false;
 		}
-- 
2.1.4

[PATCH 2/3] staging: rtl8192u: Improve code readability

[Tuomo Rinne]

Split the u4bAcParam parameter construction to multiple lines for easier
readability.

Signed-off-by: Tuomo Rinne <tuomo.rinne@gmail.com>
---
 drivers/staging/rtl8192u/r8192U_dm.c | 28 +++++++++++++++++++---------
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/rtl8192u/r8192U_dm.c b/drivers/staging/rtl8192u/r8192U_dm.c
index 94d898b..283dda4 100644
--- a/drivers/staging/rtl8192u/r8192U_dm.c
+++ b/drivers/staging/rtl8192u/r8192U_dm.c
@@ -2300,20 +2300,30 @@ static void dm_check_edca_turbo(
 		 * Restore original EDCA according to the declaration of AP.
 		 */
 		if (priv->bcurrent_turbo_EDCA) {
-			u8		u1bAIFS;
-			u32		u4bAcParam;
+			u8	u1bAIFS;
+			u32	u4bAcParam, op_limit, cw_max, cw_min;
+
 			struct ieee80211_qos_parameters *qos_parameters = &priv->ieee80211->current_network.qos_data.parameters;
 			u8 mode = priv->ieee80211->mode;
 
 			/*  For Each time updating EDCA parameter, reset EDCA turbo mode status. */
 			dm_init_edca_turbo(dev);
-			u1bAIFS = qos_parameters->aifs[0] * ((mode&(IEEE_G|IEEE_N_24G)) ? 9 : 20) + aSifsTime;
-			u4bAcParam = (((le16_to_cpu(qos_parameters->tx_op_limit[0])) << AC_PARAM_TXOP_LIMIT_OFFSET)|
-				((le16_to_cpu(qos_parameters->cw_max[0])) << AC_PARAM_ECW_MAX_OFFSET)|
-				((le16_to_cpu(qos_parameters->cw_min[0])) << AC_PARAM_ECW_MIN_OFFSET)|
-				((u32)u1bAIFS << AC_PARAM_AIFS_OFFSET));
-			/*write_nic_dword(dev, WDCAPARA_ADD[i], u4bAcParam);*/
-			write_nic_dword(dev, EDCAPARA_BE,  u4bAcParam);
+
+			u1bAIFS = qos_parameters->aifs[0] * ((mode & (IEEE_G | IEEE_N_24G)) ? 9 : 20) + aSifsTime;
+
+			op_limit = (u32)le16_to_cpu(qos_parameters->tx_op_limit[0]);
+			cw_max   = (u32)le16_to_cpu(qos_parameters->cw_max[0]);
+			cw_min   = (u32)le16_to_cpu(qos_parameters->cw_min[0]);
+
+			op_limit <<= AC_PARAM_TXOP_LIMIT_OFFSET;
+			cw_max   <<= AC_PARAM_ECW_MAX_OFFSET;
+			cw_min   <<= AC_PARAM_ECW_MIN_OFFSET;
+			u1bAIFS  <<= AC_PARAM_AIFS_OFFSET;
+
+			u4bAcParam = op_limit | cw_max | cw_min | u1bAIFS;
+
+			write_nic_dword(dev, EDCAPARA_BE, u4bAcParam);
+
 
 			/*
 			 * Check ACM bit.
-- 
2.1.4

[PATCH 3/3] staging: rtl8192u: Convert u4bAcParam to little-endian

[Tuomo Rinne]

The commit 9304b5b0d4fe ("staging: rtl8192u: Fix sparse warnings in r8192U_dm.c")
adds casting of le16 from cpu endianness. Therefore constructing
u4bAcParam potentially using big-endian order.
This patch converts the u4bAcParam parameter back to little-endian after
it has been constructed. Hence on big-endian architectures the parameter
will remain as little-endian.

Signed-off-by: Tuomo Rinne <tuomo.rinne@gmail.com>
---
 drivers/staging/rtl8192u/r8192U_dm.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/rtl8192u/r8192U_dm.c b/drivers/staging/rtl8192u/r8192U_dm.c
index 283dda4..e6f8d1d 100644
--- a/drivers/staging/rtl8192u/r8192U_dm.c
+++ b/drivers/staging/rtl8192u/r8192U_dm.c
@@ -2321,6 +2321,7 @@ static void dm_check_edca_turbo(
 			u1bAIFS  <<= AC_PARAM_AIFS_OFFSET;
 
 			u4bAcParam = op_limit | cw_max | cw_min | u1bAIFS;
+			cpu_to_le32s(&u4bAcParam);
 
 			write_nic_dword(dev, EDCAPARA_BE, u4bAcParam);
 
-- 
2.1.4

Re: [PATCH] staging: rtl8192u: Improve readability and fix sparse warnings: cast from restricted __le16

[Greg KH]

On Sat, Apr 22, 2017 at 11:55:23PM +0100, Tuomo Rinne wrote:
> This patch fixes following sparse warnings:
> 
> drivers/staging/rtl8192u//r8192U_dm.c:2307:49: warning: cast from restricted __le16
> drivers/staging/rtl8192u//r8192U_dm.c:2308:44: warning: cast from restricted __le16
> drivers/staging/rtl8192u//r8192U_dm.c:2309:44: warning: cast from restricted __le16
> 
> In order to avoid the warnings the u4bAcParam variable is constructed
> using the cpu's endianness and in case of big endian architectures the
> variable is converted back to little endian before passed to write_nic_dword
> function.
> 
> Also the patch improves readability by getting rid of unnecessary scope
> and splitting complex variable construction to multiple lines.

Don't do multiple things in the same patch, please break this up into
different patches, each one doing only one thing.

thanks,

greg k-h

[PATCH 1/3] staging: rtl8192u: Remove unnecessary scope

[Tuomo Rinne]

Remove scope unnecessary scope that is already enforced by the if
statements scope.

Signed-off-by: Tuomo Rinne <tuomo.rinne@gmail.com>
---
 drivers/staging/rtl8192u/r8192U_dm.c | 66 +++++++++++++++++-------------------
 1 file changed, 32 insertions(+), 34 deletions(-)

diff --git a/drivers/staging/rtl8192u/r8192U_dm.c b/drivers/staging/rtl8192u/r8192U_dm.c
index 623d495..16cafb62 100644
--- a/drivers/staging/rtl8192u/r8192U_dm.c
+++ b/drivers/staging/rtl8192u/r8192U_dm.c
@@ -2300,43 +2300,41 @@ static void dm_check_edca_turbo(
 		 * Restore original EDCA according to the declaration of AP.
 		 */
 		if (priv->bcurrent_turbo_EDCA) {
+			u8		u1bAIFS;
+			u32		u4bAcParam;
+			struct ieee80211_qos_parameters *qos_parameters = &priv->ieee80211->current_network.qos_data.parameters;
+			u8 mode = priv->ieee80211->mode;
+
+			/*  For Each time updating EDCA parameter, reset EDCA turbo mode status. */
+			dm_init_edca_turbo(dev);
+			u1bAIFS = qos_parameters->aifs[0] * ((mode&(IEEE_G|IEEE_N_24G)) ? 9 : 20) + aSifsTime;
+			u4bAcParam = (((le16_to_cpu(qos_parameters->tx_op_limit[0])) << AC_PARAM_TXOP_LIMIT_OFFSET)|
+				((le16_to_cpu(qos_parameters->cw_max[0])) << AC_PARAM_ECW_MAX_OFFSET)|
+				((le16_to_cpu(qos_parameters->cw_min[0])) << AC_PARAM_ECW_MIN_OFFSET)|
+				((u32)u1bAIFS << AC_PARAM_AIFS_OFFSET));
+			/*write_nic_dword(dev, WDCAPARA_ADD[i], u4bAcParam);*/
+			write_nic_dword(dev, EDCAPARA_BE,  u4bAcParam);
+
+			/*
+			 * Check ACM bit.
+			 * If it is set, immediately set ACM control bit to downgrading AC for passing WMM testplan. Annie, 2005-12-13.
+			 */
 			{
-				u8		u1bAIFS;
-				u32		u4bAcParam;
-				struct ieee80211_qos_parameters *qos_parameters = &priv->ieee80211->current_network.qos_data.parameters;
-				u8 mode = priv->ieee80211->mode;
-
-				/*  For Each time updating EDCA parameter, reset EDCA turbo mode status. */
-				dm_init_edca_turbo(dev);
-				u1bAIFS = qos_parameters->aifs[0] * ((mode&(IEEE_G|IEEE_N_24G)) ? 9 : 20) + aSifsTime;
-				u4bAcParam = (((le16_to_cpu(qos_parameters->tx_op_limit[0])) << AC_PARAM_TXOP_LIMIT_OFFSET)|
-					((le16_to_cpu(qos_parameters->cw_max[0])) << AC_PARAM_ECW_MAX_OFFSET)|
-					((le16_to_cpu(qos_parameters->cw_min[0])) << AC_PARAM_ECW_MIN_OFFSET)|
-					((u32)u1bAIFS << AC_PARAM_AIFS_OFFSET));
-				/*write_nic_dword(dev, WDCAPARA_ADD[i], u4bAcParam);*/
-				write_nic_dword(dev, EDCAPARA_BE,  u4bAcParam);
-
-				/*
-				 * Check ACM bit.
-				 * If it is set, immediately set ACM control bit to downgrading AC for passing WMM testplan. Annie, 2005-12-13.
-				 */
-				{
-					/*  TODO:  Modified this part and try to set acm control in only 1 IO processing!! */
-
-					PACI_AIFSN	pAciAifsn = (PACI_AIFSN)&(qos_parameters->aifs[0]);
-					u8		AcmCtrl;
-
-					read_nic_byte(dev, AcmHwCtrl, &AcmCtrl);
-
-					if (pAciAifsn->f.ACM) { /*  ACM bit is 1. */
-						AcmCtrl |= AcmHw_BeqEn;
-					} else {	/* ACM bit is 0. */
-						AcmCtrl &= (~AcmHw_BeqEn);
-					}
+				/*  TODO:  Modified this part and try to set acm control in only 1 IO processing!! */
 
-					RT_TRACE(COMP_QOS, "SetHwReg8190pci(): [HW_VAR_ACM_CTRL] Write 0x%X\n", AcmCtrl);
-					write_nic_byte(dev, AcmHwCtrl, AcmCtrl);
+				PACI_AIFSN	pAciAifsn = (PACI_AIFSN)&(qos_parameters->aifs[0]);
+				u8		AcmCtrl;
+
+				read_nic_byte(dev, AcmHwCtrl, &AcmCtrl);
+
+				if (pAciAifsn->f.ACM) { /*  ACM bit is 1. */
+					AcmCtrl |= AcmHw_BeqEn;
+				} else {	/* ACM bit is 0. */
+					AcmCtrl &= (~AcmHw_BeqEn);
 				}
+
+				RT_TRACE(COMP_QOS, "SetHwReg8190pci(): [HW_VAR_ACM_CTRL] Write 0x%X\n", AcmCtrl);
+				write_nic_byte(dev, AcmHwCtrl, AcmCtrl);
 			}
 			priv->bcurrent_turbo_EDCA = false;
 		}
-- 
2.1.4

[PATCH 2/3] staging: rtl8192u: Improve code readability

[Tuomo Rinne]

Split the u4bAcParam parameter construction to multiple lines for easier
readability.

Signed-off-by: Tuomo Rinne <tuomo.rinne@gmail.com>
---
 drivers/staging/rtl8192u/r8192U_dm.c | 28 +++++++++++++++++++---------
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/rtl8192u/r8192U_dm.c b/drivers/staging/rtl8192u/r8192U_dm.c
index 16cafb62..5e84ed7 100644
--- a/drivers/staging/rtl8192u/r8192U_dm.c
+++ b/drivers/staging/rtl8192u/r8192U_dm.c
@@ -2300,20 +2300,30 @@ static void dm_check_edca_turbo(
 		 * Restore original EDCA according to the declaration of AP.
 		 */
 		if (priv->bcurrent_turbo_EDCA) {
-			u8		u1bAIFS;
-			u32		u4bAcParam;
+			u8	u1bAIFS;
+			u32	u4bAcParam, op_limit, cw_max, cw_min;
+
 			struct ieee80211_qos_parameters *qos_parameters = &priv->ieee80211->current_network.qos_data.parameters;
 			u8 mode = priv->ieee80211->mode;
 
 			/*  For Each time updating EDCA parameter, reset EDCA turbo mode status. */
 			dm_init_edca_turbo(dev);
-			u1bAIFS = qos_parameters->aifs[0] * ((mode&(IEEE_G|IEEE_N_24G)) ? 9 : 20) + aSifsTime;
-			u4bAcParam = (((le16_to_cpu(qos_parameters->tx_op_limit[0])) << AC_PARAM_TXOP_LIMIT_OFFSET)|
-				((le16_to_cpu(qos_parameters->cw_max[0])) << AC_PARAM_ECW_MAX_OFFSET)|
-				((le16_to_cpu(qos_parameters->cw_min[0])) << AC_PARAM_ECW_MIN_OFFSET)|
-				((u32)u1bAIFS << AC_PARAM_AIFS_OFFSET));
-			/*write_nic_dword(dev, WDCAPARA_ADD[i], u4bAcParam);*/
-			write_nic_dword(dev, EDCAPARA_BE,  u4bAcParam);
+
+			u1bAIFS = qos_parameters->aifs[0] * ((mode & (IEEE_G | IEEE_N_24G)) ? 9 : 20) + aSifsTime;
+
+			op_limit = (u32)le16_to_cpu(qos_parameters->tx_op_limit[0]);
+			cw_max   = (u32)le16_to_cpu(qos_parameters->cw_max[0]);
+			cw_min   = (u32)le16_to_cpu(qos_parameters->cw_min[0]);
+
+			op_limit <<= AC_PARAM_TXOP_LIMIT_OFFSET;
+			cw_max   <<= AC_PARAM_ECW_MAX_OFFSET;
+			cw_min   <<= AC_PARAM_ECW_MIN_OFFSET;
+			u1bAIFS  <<= AC_PARAM_AIFS_OFFSET;
+
+			u4bAcParam = op_limit | cw_max | cw_min | u1bAIFS;
+
+			write_nic_dword(dev, EDCAPARA_BE, u4bAcParam);
+
 
 			/*
 			 * Check ACM bit.
-- 
2.1.4