* [PATCH] staging: rtl8723au: change parameter type in rtl8723a_set_rssi_cmd declaration
@ 2015-12-15 20:34 drivengroove
2015-12-16 14:51 ` Jes Sorensen
0 siblings, 1 reply; 5+ messages in thread
From: drivengroove @ 2015-12-15 20:34 UTC (permalink / raw)
To: Larry.Finger, Jes.Sorensen, gregkh; +Cc: linux-wireless, Anatoly Stepanov
From: Anatoly Stepanov <drivengroove@gmail.com>
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 <drivengroove@gmail.com>
---
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(-)
diff --git a/drivers/staging/rtl8723au/hal/odm.c b/drivers/staging/rtl8723au/hal/odm.c
index 6b9dbef..6fed13e 100644
--- a/drivers/staging/rtl8723au/hal/odm.c
+++ b/drivers/staging/rtl8723au/hal/odm.c
@@ -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;
diff --git a/drivers/staging/rtl8723au/hal/rtl8723a_cmd.c b/drivers/staging/rtl8723au/hal/rtl8723a_cmd.c
index 1662c03c..e899d05 100644
--- a/drivers/staging/rtl8723au/hal/rtl8723a_cmd.c
+++ b/drivers/staging/rtl8723au/hal/rtl8723a_cmd.c
@@ -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;
}
diff --git a/drivers/staging/rtl8723au/include/rtl8723a_cmd.h b/drivers/staging/rtl8723au/include/rtl8723a_cmd.h
index 014c02e..e39e38a 100644
--- a/drivers/staging/rtl8723au/include/rtl8723a_cmd.h
+++ b/drivers/staging/rtl8723au/include/rtl8723a_cmd.h
@@ -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);
--
1.9.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] staging: rtl8723au: change parameter type in rtl8723a_set_rssi_cmd declaration
2015-12-15 20:34 [PATCH] staging: rtl8723au: change parameter type in rtl8723a_set_rssi_cmd declaration drivengroove
@ 2015-12-16 14:51 ` Jes Sorensen
2015-12-17 13:47 ` [PATCH v2 0/1] " drivengroove
0 siblings, 1 reply; 5+ messages in thread
From: Jes Sorensen @ 2015-12-16 14:51 UTC (permalink / raw)
To: drivengroove; +Cc: Larry.Finger, gregkh, linux-wireless
drivengroove@gmail.com writes:
> From: Anatoly Stepanov <drivengroove@gmail.com>
>
> 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 <drivengroove@gmail.com>
> ---
> 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(-)
>
> diff --git a/drivers/staging/rtl8723au/hal/odm.c b/drivers/staging/rtl8723au/hal/odm.c
> index 6b9dbef..6fed13e 100644
> --- a/drivers/staging/rtl8723au/hal/odm.c
> +++ b/drivers/staging/rtl8723au/hal/odm.c
> @@ -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;
> diff --git a/drivers/staging/rtl8723au/hal/rtl8723a_cmd.c b/drivers/staging/rtl8723au/hal/rtl8723a_cmd.c
> index 1662c03c..e899d05 100644
> --- a/drivers/staging/rtl8723au/hal/rtl8723a_cmd.c
> +++ b/drivers/staging/rtl8723au/hal/rtl8723a_cmd.c
> @@ -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;
> }
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
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH v2 0/1] staging: rtl8723au: change parameter type in rtl8723a_set_rssi_cmd declaration
2015-12-16 14:51 ` Jes Sorensen
@ 2015-12-17 13:47 ` drivengroove
2015-12-17 13:47 ` [PATCH v2 1/1] " drivengroove
0 siblings, 1 reply; 5+ messages in thread
From: drivengroove @ 2015-12-17 13:47 UTC (permalink / raw)
To: Larry.Finger, Jes.Sorensen, gregkh; +Cc: linux-wireless, Anatoly Stepanov
From: Anatoly Stepanov <drivengroove@gmail.com>
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(-)
--
1.9.1
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH v2 1/1] staging: rtl8723au: change parameter type in rtl8723a_set_rssi_cmd declaration
2015-12-17 13:47 ` [PATCH v2 0/1] " drivengroove
@ 2015-12-17 13:47 ` drivengroove
2015-12-17 14:59 ` Jes Sorensen
0 siblings, 1 reply; 5+ messages in thread
From: drivengroove @ 2015-12-17 13:47 UTC (permalink / raw)
To: Larry.Finger, Jes.Sorensen, gregkh; +Cc: linux-wireless, Anatoly Stepanov
From: Anatoly Stepanov <drivengroove@gmail.com>
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 <drivengroove@gmail.com>
---
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(-)
diff --git a/drivers/staging/rtl8723au/hal/odm.c b/drivers/staging/rtl8723au/hal/odm.c
index 6b9dbef..45d70fd 100644
--- a/drivers/staging/rtl8723au/hal/odm.c
+++ b/drivers/staging/rtl8723au/hal/odm.c
@@ -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;
diff --git a/drivers/staging/rtl8723au/hal/rtl8723a_cmd.c b/drivers/staging/rtl8723au/hal/rtl8723a_cmd.c
index 1662c03c..2230f4c 100644
--- a/drivers/staging/rtl8723au/hal/rtl8723a_cmd.c
+++ b/drivers/staging/rtl8723au/hal/rtl8723a_cmd.c
@@ -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;
}
diff --git a/drivers/staging/rtl8723au/include/rtl8723a_cmd.h b/drivers/staging/rtl8723au/include/rtl8723a_cmd.h
index 014c02e..f95535a 100644
--- a/drivers/staging/rtl8723au/include/rtl8723a_cmd.h
+++ b/drivers/staging/rtl8723au/include/rtl8723a_cmd.h
@@ -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);
--
1.9.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v2 1/1] staging: rtl8723au: change parameter type in rtl8723a_set_rssi_cmd declaration
2015-12-17 13:47 ` [PATCH v2 1/1] " drivengroove
@ 2015-12-17 14:59 ` Jes Sorensen
0 siblings, 0 replies; 5+ messages in thread
From: Jes Sorensen @ 2015-12-17 14:59 UTC (permalink / raw)
To: drivengroove; +Cc: Larry.Finger, gregkh, linux-wireless
drivengroove@gmail.com writes:
> From: Anatoly Stepanov <drivengroove@gmail.com>
>
> 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 <drivengroove@gmail.com>
> ---
> 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 <Jes.Sorensen@redhat.com>
>
> diff --git a/drivers/staging/rtl8723au/hal/odm.c b/drivers/staging/rtl8723au/hal/odm.c
> index 6b9dbef..45d70fd 100644
> --- a/drivers/staging/rtl8723au/hal/odm.c
> +++ b/drivers/staging/rtl8723au/hal/odm.c
> @@ -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;
> diff --git a/drivers/staging/rtl8723au/hal/rtl8723a_cmd.c b/drivers/staging/rtl8723au/hal/rtl8723a_cmd.c
> index 1662c03c..2230f4c 100644
> --- a/drivers/staging/rtl8723au/hal/rtl8723a_cmd.c
> +++ b/drivers/staging/rtl8723au/hal/rtl8723a_cmd.c
> @@ -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;
> }
> diff --git a/drivers/staging/rtl8723au/include/rtl8723a_cmd.h b/drivers/staging/rtl8723au/include/rtl8723a_cmd.h
> index 014c02e..f95535a 100644
> --- a/drivers/staging/rtl8723au/include/rtl8723a_cmd.h
> +++ b/drivers/staging/rtl8723au/include/rtl8723a_cmd.h
> @@ -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);
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-12-17 14:59 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-12-15 20:34 [PATCH] staging: rtl8723au: change parameter type in rtl8723a_set_rssi_cmd declaration drivengroove
2015-12-16 14:51 ` Jes Sorensen
2015-12-17 13:47 ` [PATCH v2 0/1] " drivengroove
2015-12-17 13:47 ` [PATCH v2 1/1] " drivengroove
2015-12-17 14:59 ` Jes Sorensen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).