Re: [PATCH 5/5] staging: r8188eu: turbo scan is always off for r8188eu

[Pavel Skripkin]

On 1/2/22 20:59, Martin Kaiser wrote:
> Turbo scan is always disabled. Remove the TurboScanOff variable and
> related checks.
>
> Signed-off-by: Martin Kaiser <martin@xxxxxxxxx>
> ---
>   drivers/staging/r8188eu/hal/rtl8188e_rf6052.c | 21 +++++++------------
>   1 file changed, 7 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/staging/r8188eu/hal/rtl8188e_rf6052.c b/drivers/staging/r8188eu/hal/rtl8188e_rf6052.c
> index 0b982cc7c86f..6e0231099986 100644
> --- a/drivers/staging/r8188eu/hal/rtl8188e_rf6052.c
> +++ b/drivers/staging/r8188eu/hal/rtl8188e_rf6052.c
> @@ -87,28 +87,21 @@ rtl8188e_PHY_RF6052SetCckTxPower(
>   	struct hal_data_8188e *pHalData = &Adapter->haldata;
>   	struct mlme_ext_priv *pmlmeext = &Adapter->mlmeextpriv;
>   	u32 TxAGC[2] = {0, 0}, tmpval = 0, pwrtrac_value;
> -	bool TurboScanOff = false;
>   	u8 idx1, idx2;
>   	u8 *ptr;
>   	u8 direction;
> -	/* FOR CE ,must disable turbo scan */
> -	TurboScanOff = true;
>   
>   	if (pmlmeext->sitesurvey_res.state == SCAN_PROCESS) {
>   		TxAGC[RF_PATH_A] = 0x3f3f3f3f;
>   		TxAGC[RF_PATH_B] = 0x3f3f3f3f;
>   
> -		TurboScanOff = true;/* disable turbo scan */
> -
> -		if (TurboScanOff) {
> -			for (idx1 = RF_PATH_A; idx1 <= RF_PATH_B; idx1++) {
> -				TxAGC[idx1] =
> -					pPowerlevel[idx1] | (pPowerlevel[idx1] << 8) |
> -					(pPowerlevel[idx1] << 16) | (pPowerlevel[idx1] << 24);
> -				/*  2010/10/18 MH For external PA module. We need to limit power index to be less than 0x20. */
> -				if (TxAGC[idx1] > 0x20 && pHalData->ExternalPA)
> -					TxAGC[idx1] = 0x20;
> -			}
> +		for (idx1 = RF_PATH_A; idx1 <= RF_PATH_B; idx1++) {
> +			TxAGC[idx1] =
> +				pPowerlevel[idx1] | (pPowerlevel[idx1] << 8) |
> +				(pPowerlevel[idx1] << 16) | (pPowerlevel[idx1] << 24);
> +			/*  2010/10/18 MH For external PA module. We need to limit power index to be less than 0x20. */
> +			if (TxAGC[idx1] > 0x20 && pHalData->ExternalPA)
> +				TxAGC[idx1] = 0x20;
>   		}
>   	} else {
>   		for (idx1 = RF_PATH_A; idx1 <= RF_PATH_B; idx1++) {

Hi Martin,

thanks for your clean up work!


It's a bit unrelated to the patch, but I found it while reviewing this. 
It's in the same function rtl8188e_PHY_RF6052SetCckTxPower():

89: 	u32 TxAGC[2] = {0, 0};
...
92	u8 *ptr;
...
129	for (idx1 = RF_PATH_A; idx1 <= RF_PATH_B; idx1++) {
130		ptr = (u8 *)(&TxAGC[idx1]);
131		for (idx2 = 0; idx2 < 4; idx2++) {
132			if (*ptr > RF6052_MAX_TX_PWR)
133				*ptr = RF6052_MAX_TX_PWR;
134			ptr++;
135		}
136	}


What is going on here? Code just checks and writes to random place on 
stack outside TxAGC array? I might be missing something, but it looks 
wrong...

Just wanted to share, since I believe your knowledge about this driver 
is much deeper than mine :)

thanks


With regards,
Pavel Skripkin