staging: rtl8723au: change parameter type in rtl8723a_set_rssi_cmd declaration

PATCH Superseded [email protected] 2 participants 5 messages Dec 15, 2015 – Dec 17, 2015 Staging

lore

Share & tools

lore

[email protected]

author

2015-12-15 20:35 UTC #0 lore raw

PATCH 1/1 staging: rtl8723au: change parameter type in rtl8723a_set_rssi_cmd declaration

From: Anatoly Stepanov <[email protected]>

Previosly the function had the following prototype:
int rtl8723a_set_rssi_cmd(struct rtw_adapter *padapter, u8 *param)

My suggestion here is to modify the prototype this way:
int rtl8723a_set_rssi_cmd(struct rtw_adapter *padapter, u32 *param)

We can do this based on the following considerations:
1. rtl8723a_set_rssi_cmd is used only with 32-bit "param" values
2. There's no point in using "__u8 *param" until we pass param length

Signed-off-by Anatoly Stepanov <[email protected]>

---
 drivers/staging/rtl8723au/hal/odm.c              | 2 +-
 drivers/staging/rtl8723au/hal/rtl8723a_cmd.c     | 6 +++---
 drivers/staging/rtl8723au/include/rtl8723a_cmd.h | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

drivers/staging/rtl8723au/hal/odm.c +1 -1

index 6b9dbef..6fed13e 100644
@@ -1274,7 +1274,7 @@ static void odm_RSSIMonitorCheck(struct dm_odm_t *pDM_Odm)
 
 	for (i = 0; i < sta_cnt; i++) {
 		if (PWDB_rssi[i] != (0))
-			rtl8723a_set_rssi_cmd(Adapter, (u8 *)&PWDB_rssi[i]);
+			rtl8723a_set_rssi_cmd(Adapter, &PWDB_rssi[i]);
 	}
 
 	pdmpriv->EntryMaxUndecoratedSmoothedPWDB = MaxDB;

drivers/staging/rtl8723au/hal/rtl8723a_cmd.c +3 -3

index 1662c03c..e899d05 100644
@@ -113,11 +113,11 @@ exit:
 	return ret;
 }
 
-int rtl8723a_set_rssi_cmd(struct rtw_adapter *padapter, u8 *param)
+int rtl8723a_set_rssi_cmd(struct rtw_adapter *padapter, u32 *param)
 {
-	*((u32 *)param) = cpu_to_le32(*((u32 *)param));
+	__le32 cmd = cpu_to_le32(*param);
 
-	FillH2CCmd(padapter, RSSI_SETTING_EID, 3, param);
+	FillH2CCmd(padapter, RSSI_SETTING_EID, 3, (void *)&cmd);
 
 	return _SUCCESS;
 }

drivers/staging/rtl8723au/include/rtl8723a_cmd.h +1 -1

index 014c02e..e39e38a 100644
@@ -149,7 +149,7 @@ void rtl8723a_set_BTCoex_AP_mode_FwRsvdPkt_cmd(struct rtw_adapter *padapter);
 #else
 #define rtl8723a_set_BTCoex_AP_mode_FwRsvdPkt_cmd(padapter) do {} while(0)
 #endif
-int rtl8723a_set_rssi_cmd(struct rtw_adapter *padapter, u8 *param);
+int rtl8723a_set_rssi_cmd(struct rtw_adapter *padapter, u32 *param);
 int rtl8723a_set_raid_cmd(struct rtw_adapter *padapter, u32 mask, u8 arg);
 void rtl8723a_add_rateatid(struct rtw_adapter *padapter, u32 bitmap, u8 arg, u8 rssi_level);

Signature · 1 line

1.9.1

Jes Sorensen

2015-12-16 14:51 UTC #1 lore raw

Quoted 41 lines · [email protected] writes:

From: Anatoly Stepanov <[email protected]>
Previosly the function had the following prototype:
int rtl8723a_set_rssi_cmd(struct rtw_adapter *padapter, u8 *param)
My suggestion here is to modify the prototype this way:
int rtl8723a_set_rssi_cmd(struct rtw_adapter *padapter, u32 *param)
We can do this based on the following considerations:
1. rtl8723a_set_rssi_cmd is used only with 32-bit "param" values
2. There's no point in using "__u8 *param" until we pass param length
Signed-off-by Anatoly Stepanov <[email protected]>
---
 drivers/staging/rtl8723au/hal/odm.c              | 2 +-
 drivers/staging/rtl8723au/hal/rtl8723a_cmd.c     | 6 +++---
 drivers/staging/rtl8723au/include/rtl8723a_cmd.h | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)
drivers/staging/rtl8723au/hal/odm.c +1 -1
index 6b9dbef..6fed13e 100644
@@ -1274,7 +1274,7 @@ static void odm_RSSIMonitorCheck(struct dm_odm_t *pDM_Odm)
 
 	for (i = 0; i < sta_cnt; i++) {
 		if (PWDB_rssi[i] != (0))
-			rtl8723a_set_rssi_cmd(Adapter, (u8 *)&PWDB_rssi[i]);
+			rtl8723a_set_rssi_cmd(Adapter, &PWDB_rssi[i]);
 	}
 
 	pdmpriv->EntryMaxUndecoratedSmoothedPWDB = MaxDB;
drivers/staging/rtl8723au/hal/rtl8723a_cmd.c +0 -1
index 1662c03c..e899d05 100644
@@ -113,11 +113,11 @@ exit:
 	return ret;
 }
 
-int rtl8723a_set_rssi_cmd(struct rtw_adapter *padapter, u8 *param)

drivers/staging/rtl8723au/hal/rtl8723a_cmd.c exit: · line 123

Quoted 10 lines

drivers/staging/rtl8723au/hal/rtl8723a_cmd.c +3 -2

+int rtl8723a_set_rssi_cmd(struct rtw_adapter *padapter, u32 *param)
 {
-	*((u32 *)param) = cpu_to_le32(*((u32 *)param));
+	__le32 cmd = cpu_to_le32(*param);
 
-	FillH2CCmd(padapter, RSSI_SETTING_EID, 3, param);
+	FillH2CCmd(padapter, RSSI_SETTING_EID, 3, (void *)&cmd);
 
 	return _SUCCESS;
 }

This is a step in the right direction, but lets get it right the first time. There really is little reason for passing this by reference when it should suffice passing by value.

Cheers,
Jes

[email protected]

author

2015-12-17 13:48 UTC #2 lore raw

v2 cover staging: rtl8723au: change parameter type in rtl8723a_set_rssi_cmd declaration

From: Anatoly Stepanov <[email protected]>

Hi, Jes!

I updated the patch, according to your idea, now "param" is passed by value.

Anatoly Stepanov (1):
staging: rtl8723au: change parameter type in rtl8723a_set_rssi_cmd
declaration

 drivers/staging/rtl8723au/hal/odm.c              | 2 +-
 drivers/staging/rtl8723au/hal/rtl8723a_cmd.c     | 6 +++---
 drivers/staging/rtl8723au/include/rtl8723a_cmd.h | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

Signature · 1 line

1.9.1

[email protected]

author

2015-12-17 13:48 UTC #3 lore raw

PATCH v2 1/1 staging: rtl8723au: change parameter type in rtl8723a_set_rssi_cmd declaration

From: Anatoly Stepanov <[email protected]>

Previosly the function had the following prototype:
int rtl8723a_set_rssi_cmd(struct rtw_adapter *padapter, u8 *param)

My suggestion here is to modify the prototype this way:
int rtl8723a_set_rssi_cmd(struct rtw_adapter *padapter, u32 param)

We can do this based on the following considerations:
1. rtl8723a_set_rssi_cmd is used only with 32-bit "param" values
2. There's no point in using "u8 *param" until we pass param length
3. As we just read "param", it's ok to pass it by value

Signed-off-by Anatoly Stepanov <[email protected]>

---
 drivers/staging/rtl8723au/hal/odm.c              | 2 +-
 drivers/staging/rtl8723au/hal/rtl8723a_cmd.c     | 6 +++---
 drivers/staging/rtl8723au/include/rtl8723a_cmd.h | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

drivers/staging/rtl8723au/hal/odm.c +1 -1

index 6b9dbef..45d70fd 100644
@@ -1274,7 +1274,7 @@ static void odm_RSSIMonitorCheck(struct dm_odm_t *pDM_Odm)
 
 	for (i = 0; i < sta_cnt; i++) {
 		if (PWDB_rssi[i] != (0))
-			rtl8723a_set_rssi_cmd(Adapter, (u8 *)&PWDB_rssi[i]);
+			rtl8723a_set_rssi_cmd(Adapter, PWDB_rssi[i]);
 	}
 
 	pdmpriv->EntryMaxUndecoratedSmoothedPWDB = MaxDB;

drivers/staging/rtl8723au/hal/rtl8723a_cmd.c +3 -3

index 1662c03c..2230f4c 100644
@@ -113,11 +113,11 @@ exit:
 	return ret;
 }
 
-int rtl8723a_set_rssi_cmd(struct rtw_adapter *padapter, u8 *param)
+int rtl8723a_set_rssi_cmd(struct rtw_adapter *padapter, u32 param)
 {
-	*((u32 *)param) = cpu_to_le32(*((u32 *)param));
+	__le32 cmd = cpu_to_le32(param);
 
-	FillH2CCmd(padapter, RSSI_SETTING_EID, 3, param);
+	FillH2CCmd(padapter, RSSI_SETTING_EID, 3, (void *)&cmd);
 
 	return _SUCCESS;
 }

drivers/staging/rtl8723au/include/rtl8723a_cmd.h +1 -1

index 014c02e..f95535a 100644
@@ -149,7 +149,7 @@ void rtl8723a_set_BTCoex_AP_mode_FwRsvdPkt_cmd(struct rtw_adapter *padapter);
 #else
 #define rtl8723a_set_BTCoex_AP_mode_FwRsvdPkt_cmd(padapter) do {} while(0)
 #endif
-int rtl8723a_set_rssi_cmd(struct rtw_adapter *padapter, u8 *param);
+int rtl8723a_set_rssi_cmd(struct rtw_adapter *padapter, u32 param);
 int rtl8723a_set_raid_cmd(struct rtw_adapter *padapter, u32 mask, u8 arg);
 void rtl8723a_add_rateatid(struct rtw_adapter *padapter, u32 bitmap, u8 arg, u8 rssi_level);

Signature · 1 line

1.9.1

Jes Sorensen

Acked-by on v2 1/1

2015-12-17 14:59 UTC #4 lore raw

Quoted 13 lines · [email protected] writes:

From: Anatoly Stepanov <[email protected]>
Previosly the function had the following prototype:
int rtl8723a_set_rssi_cmd(struct rtw_adapter *padapter, u8 *param)
My suggestion here is to modify the prototype this way:
int rtl8723a_set_rssi_cmd(struct rtw_adapter *padapter, u32 param)
We can do this based on the following considerations:
1. rtl8723a_set_rssi_cmd is used only with 32-bit "param" values
2. There's no point in using "u8 *param" until we pass param length
3. As we just read "param", it's ok to pass it by value

Quoted 6 lines

Signed-off-by Anatoly Stepanov <[email protected]>

---
 drivers/staging/rtl8723au/hal/odm.c              | 2 +-
 drivers/staging/rtl8723au/hal/rtl8723a_cmd.c     | 6 +++---
 drivers/staging/rtl8723au/include/rtl8723a_cmd.h | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

Looks good to me!

Acked-by Jes Sorensen <[email protected]>

Quoted 45 lines

drivers/staging/rtl8723au/hal/odm.c +1 -1

index 6b9dbef..45d70fd 100644
@@ -1274,7 +1274,7 @@ static void odm_RSSIMonitorCheck(struct dm_odm_t *pDM_Odm)
 
 	for (i = 0; i < sta_cnt; i++) {
 		if (PWDB_rssi[i] != (0))
-			rtl8723a_set_rssi_cmd(Adapter, (u8 *)&PWDB_rssi[i]);
+			rtl8723a_set_rssi_cmd(Adapter, PWDB_rssi[i]);
 	}
 
 	pdmpriv->EntryMaxUndecoratedSmoothedPWDB = MaxDB;

drivers/staging/rtl8723au/hal/rtl8723a_cmd.c +3 -3

index 1662c03c..2230f4c 100644
@@ -113,11 +113,11 @@ exit:
 	return ret;
 }
 
-int rtl8723a_set_rssi_cmd(struct rtw_adapter *padapter, u8 *param)
+int rtl8723a_set_rssi_cmd(struct rtw_adapter *padapter, u32 param)
 {
-	*((u32 *)param) = cpu_to_le32(*((u32 *)param));
+	__le32 cmd = cpu_to_le32(param);
 
-	FillH2CCmd(padapter, RSSI_SETTING_EID, 3, param);
+	FillH2CCmd(padapter, RSSI_SETTING_EID, 3, (void *)&cmd);
 
 	return _SUCCESS;
 }

drivers/staging/rtl8723au/include/rtl8723a_cmd.h +1 -1

index 014c02e..f95535a 100644
@@ -149,7 +149,7 @@ void rtl8723a_set_BTCoex_AP_mode_FwRsvdPkt_cmd(struct rtw_adapter *padapter);
 #else
 #define rtl8723a_set_BTCoex_AP_mode_FwRsvdPkt_cmd(padapter) do {} while(0)
 #endif
-int rtl8723a_set_rssi_cmd(struct rtw_adapter *padapter, u8 *param);
+int rtl8723a_set_rssi_cmd(struct rtw_adapter *padapter, u32 param);
 int rtl8723a_set_raid_cmd(struct rtw_adapter *padapter, u32 mask, u8 arg);
 void rtl8723a_add_rateatid(struct rtw_adapter *padapter, u32 bitmap, u8 arg, u8 rssi_level);