diff mbox series

wifi: rtlwifi: rtl8192de: Fix byte order of chip version

Message ID 81b6c452-e940-423a-acf7-4a7b7c5e7847@gmail.com
State New
Headers show
Series wifi: rtlwifi: rtl8192de: Fix byte order of chip version | expand

Commit Message

Bitterblue Smith Jan. 12, 2024, 10:50 p.m. UTC
The chip version stored in the efuse is currently assumed to be in
big endian order:

#define EEPROME_CHIP_VERSION_L			0x3FF
#define EEPROME_CHIP_VERSION_H			0x3FE

But other 2-byte things in the efuse are stored in little endian order.
For example, the EEPROM ID, the vendor ID, the product ID.

The out-of-kernel driver for the USB version of the chip uses the same
macros and version detection code as this driver. They recognise
0xaa55, 0x9966, and 0xcc33 as correct versions. With the original
macros, my device's version is the unrecognised value of 0x33cc. This
seems like a mistake.

Swap the addresses to fix the chip version detection.

Compile tested only.

Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>
---
 drivers/net/wireless/realtek/rtlwifi/rtl8192de/reg.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Ping-Ke Shih Jan. 15, 2024, 1:55 a.m. UTC | #1
> -----Original Message-----
> From: Bitterblue Smith <rtl8821cerfe2@gmail.com>
> Sent: Saturday, January 13, 2024 6:50 AM
> To: linux-wireless@vger.kernel.org
> Cc: Ping-Ke Shih <pkshih@realtek.com>; Larry Finger <Larry.Finger@lwfinger.net>
> Subject: [PATCH] wifi: rtlwifi: rtl8192de: Fix byte order of chip version
> 
> The chip version stored in the efuse is currently assumed to be in
> big endian order:
> 
> #define EEPROME_CHIP_VERSION_L                  0x3FF
> #define EEPROME_CHIP_VERSION_H                  0x3FE
> 
> But other 2-byte things in the efuse are stored in little endian order.
> For example, the EEPROM ID, the vendor ID, the product ID.
> 
> The out-of-kernel driver for the USB version of the chip uses the same
> macros and version detection code as this driver. They recognise
> 0xaa55, 0x9966, and 0xcc33 as correct versions. With the original
> macros, my device's version is the unrecognised value of 0x33cc. This
> seems like a mistake.

I will check this internally. Please wait a while.
Ping-Ke Shih Jan. 15, 2024, 5:42 a.m. UTC | #2
> -----Original Message-----
> From: Bitterblue Smith <rtl8821cerfe2@gmail.com>
> Sent: Saturday, January 13, 2024 6:50 AM
> To: linux-wireless@vger.kernel.org
> Cc: Ping-Ke Shih <pkshih@realtek.com>; Larry Finger <Larry.Finger@lwfinger.net>
> Subject: [PATCH] wifi: rtlwifi: rtl8192de: Fix byte order of chip version
> 
> The chip version stored in the efuse is currently assumed to be in
> big endian order:
> 
> #define EEPROME_CHIP_VERSION_L                  0x3FF
> #define EEPROME_CHIP_VERSION_H                  0x3FE
> 
> But other 2-byte things in the efuse are stored in little endian order.
> For example, the EEPROM ID, the vendor ID, the product ID.
> 
> The out-of-kernel driver for the USB version of the chip uses the same
> macros and version detection code as this driver.

I run vendor driver with 8192DU, and got correct 0xcc33. 

efuse[EEPROME_CHIP_VERSION_H] = efuse[0x3fe] = cutvalue[1] = 0xcc
efuse[EEPROME_CHIP_VERSION_L] = efuse[0x3ff] = cutvalue[0] = 0x33

So, 

chhipvalue = (cutvalue[1] << 8) | cutvalue[0] = (0xcc << 8) | 0x33 = 0xcc33;

> They recognise
> 0xaa55, 0x9966, and 0xcc33 as correct versions. With the original
> macros, my device's version is the unrecognised value of 0x33cc. This
> seems like a mistake.

Can you check the efuse value you read out?
Bitterblue Smith Jan. 15, 2024, 12:50 p.m. UTC | #3
On 15/01/2024 07:42, Ping-Ke Shih wrote:
> 
> 
>> -----Original Message-----
>> From: Bitterblue Smith <rtl8821cerfe2@gmail.com>
>> Sent: Saturday, January 13, 2024 6:50 AM
>> To: linux-wireless@vger.kernel.org
>> Cc: Ping-Ke Shih <pkshih@realtek.com>; Larry Finger <Larry.Finger@lwfinger.net>
>> Subject: [PATCH] wifi: rtlwifi: rtl8192de: Fix byte order of chip version
>>
>> The chip version stored in the efuse is currently assumed to be in
>> big endian order:
>>
>> #define EEPROME_CHIP_VERSION_L                  0x3FF
>> #define EEPROME_CHIP_VERSION_H                  0x3FE
>>
>> But other 2-byte things in the efuse are stored in little endian order.
>> For example, the EEPROM ID, the vendor ID, the product ID.
>>
>> The out-of-kernel driver for the USB version of the chip uses the same
>> macros and version detection code as this driver.
> 
> I run vendor driver with 8192DU, and got correct 0xcc33. 
> 
> efuse[EEPROME_CHIP_VERSION_H] = efuse[0x3fe] = cutvalue[1] = 0xcc
> efuse[EEPROME_CHIP_VERSION_L] = efuse[0x3ff] = cutvalue[0] = 0x33
> 
> So, 
> 
> chhipvalue = (cutvalue[1] << 8) | cutvalue[0] = (0xcc << 8) | 0x33 = 0xcc33;
> 
>> They recognise
>> 0xaa55, 0x9966, and 0xcc33 as correct versions. With the original
>> macros, my device's version is the unrecognised value of 0x33cc. This
>> seems like a mistake.
> 
> Can you check the efuse value you read out?
> 
> 

I checked again like this:

diff --git a/hal/rtl8192d_hal_init.c b/hal/rtl8192d_hal_init.c
index 156541b..175c856 100644
--- a/hal/rtl8192d_hal_init.c
+++ b/hal/rtl8192d_hal_init.c
@@ -1565,6 +1565,8 @@ hal_EfuseUpdateNormalChipVersion_92D(
 	ReadEFuseByte(Adapter,EEPROME_CHIP_VERSION_L,&CutValue[0], _FALSE);
 
 	ChipValue= (CutValue[1]<<8)|CutValue[0];
+
+	pr_err("%s: EEPROME_CHIP_VERSION_H: %#x EEPROME_CHIP_VERSION_L: %#x CutValue[1]: %#x CutValue[0]: %#x ChipValue: %#x\n", __func__, EEPROME_CHIP_VERSION_H, EEPROME_CHIP_VERSION_L, CutValue[1], CutValue[0], ChipValue);
 	switch(ChipValue){
 		case 0xAA55:
 			//ChipVer |= CHIP_92D_C_CUT;

This is the output:

Jan 15 14:35:21 ideapad2 kernel: hal_EfuseUpdateNormalChipVersion_92D: EEPROME_CHIP_VERSION_H: 0x3fe EEPROME_CHIP_VERSION_L: 0x3ff CutValue[1]: 0x33 CutValue[0]: 0xcc ChipValue: 0x33cc

I'm using branch v4.0.10:
https://github.com/lwfinger/rtl8192du/tree/v4.0.10
because the master branch doesn't work. I don't think it
matters because this code is the same in both branches.

Maybe my device really is a different version.
Ping-Ke Shih Jan. 16, 2024, 12:56 a.m. UTC | #4
> -----Original Message-----
> From: Bitterblue Smith <rtl8821cerfe2@gmail.com>
> Sent: Monday, January 15, 2024 8:51 PM
> To: Ping-Ke Shih <pkshih@realtek.com>; linux-wireless@vger.kernel.org
> Cc: Larry Finger <Larry.Finger@lwfinger.net>
> Subject: Re: [PATCH] wifi: rtlwifi: rtl8192de: Fix byte order of chip version
> 
> 
> diff --git a/hal/rtl8192d_hal_init.c b/hal/rtl8192d_hal_init.c
> index 156541b..175c856 100644
> --- a/hal/rtl8192d_hal_init.c
> +++ b/hal/rtl8192d_hal_init.c
> @@ -1565,6 +1565,8 @@ hal_EfuseUpdateNormalChipVersion_92D(
>         ReadEFuseByte(Adapter,EEPROME_CHIP_VERSION_L,&CutValue[0], _FALSE);
> 
>         ChipValue= (CutValue[1]<<8)|CutValue[0];
> +
> +       pr_err("%s: EEPROME_CHIP_VERSION_H: %#x EEPROME_CHIP_VERSION_L: %#x CutValue[1]: %#x CutValue[0]:
> %#x ChipValue: %#x\n", __func__, EEPROME_CHIP_VERSION_H, EEPROME_CHIP_VERSION_L, CutValue[1], CutValue[0],
> ChipValue);
>         switch(ChipValue){
>                 case 0xAA55:
>                         //ChipVer |= CHIP_92D_C_CUT;
> 
> This is the output:
> 
> Jan 15 14:35:21 ideapad2 kernel: hal_EfuseUpdateNormalChipVersion_92D: EEPROME_CHIP_VERSION_H: 0x3fe
> EEPROME_CHIP_VERSION_L: 0x3ff CutValue[1]: 0x33 CutValue[0]: 0xcc ChipValue: 0x33cc

With the same branch and the same changes you mentioned, output is: 

hal_EfuseUpdateNormalChipVersion_92D: EEPROME_CHIP_VERSION_H: 0x3fe EEPROME_CHIP_VERSION_L: 0x3ff CutValue[1]: 0xcc CutValue[0]: 0x33 ChipValue: 0xcc33

> 
> Maybe my device really is a different version.

Not sure what happens. I feel no one can remember the definition of these values. 
Maybe, we can just add an new value 0x33CC, and test if it works normal. 

diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192de/hw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8192de/hw.c
index 743ac6871bf4..c336d4b362f5 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8192de/hw.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192de/hw.c
@@ -1684,6 +1684,7 @@ static void _rtl92de_efuse_update_chip_version(struct ieee80211_hw *hw)
                rtl_dbg(rtlpriv, COMP_INIT, DBG_LOUD, "D-CUT!!!\n");
                break;
        case 0xCC33:
+       case 0x33CC:
                chipver |= CHIP_92D_E_CUT;
                rtl_dbg(rtlpriv, COMP_INIT, DBG_LOUD, "E-CUT!!!\n");
                break;

How did you find this weird value? Vendor driver doesn't work for you?

Ping-Ke
Bitterblue Smith Jan. 16, 2024, 3:37 p.m. UTC | #5
On 16/01/2024 02:56, Ping-Ke Shih wrote:
> 
> 
>> -----Original Message-----
>> From: Bitterblue Smith <rtl8821cerfe2@gmail.com>
>> Sent: Monday, January 15, 2024 8:51 PM
>> To: Ping-Ke Shih <pkshih@realtek.com>; linux-wireless@vger.kernel.org
>> Cc: Larry Finger <Larry.Finger@lwfinger.net>
>> Subject: Re: [PATCH] wifi: rtlwifi: rtl8192de: Fix byte order of chip version
>>
>>
>> diff --git a/hal/rtl8192d_hal_init.c b/hal/rtl8192d_hal_init.c
>> index 156541b..175c856 100644
>> --- a/hal/rtl8192d_hal_init.c
>> +++ b/hal/rtl8192d_hal_init.c
>> @@ -1565,6 +1565,8 @@ hal_EfuseUpdateNormalChipVersion_92D(
>>         ReadEFuseByte(Adapter,EEPROME_CHIP_VERSION_L,&CutValue[0], _FALSE);
>>
>>         ChipValue= (CutValue[1]<<8)|CutValue[0];
>> +
>> +       pr_err("%s: EEPROME_CHIP_VERSION_H: %#x EEPROME_CHIP_VERSION_L: %#x CutValue[1]: %#x CutValue[0]:
>> %#x ChipValue: %#x\n", __func__, EEPROME_CHIP_VERSION_H, EEPROME_CHIP_VERSION_L, CutValue[1], CutValue[0],
>> ChipValue);
>>         switch(ChipValue){
>>                 case 0xAA55:
>>                         //ChipVer |= CHIP_92D_C_CUT;
>>
>> This is the output:
>>
>> Jan 15 14:35:21 ideapad2 kernel: hal_EfuseUpdateNormalChipVersion_92D: EEPROME_CHIP_VERSION_H: 0x3fe
>> EEPROME_CHIP_VERSION_L: 0x3ff CutValue[1]: 0x33 CutValue[0]: 0xcc ChipValue: 0x33cc
> 
> With the same branch and the same changes you mentioned, output is: 
> 
> hal_EfuseUpdateNormalChipVersion_92D: EEPROME_CHIP_VERSION_H: 0x3fe EEPROME_CHIP_VERSION_L: 0x3ff CutValue[1]: 0xcc CutValue[0]: 0x33 ChipValue: 0xcc33
> 
>>
>> Maybe my device really is a different version.
> 
> Not sure what happens. I feel no one can remember the definition of these values. 
> Maybe, we can just add an new value 0x33CC, and test if it works normal. 
> 
> diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192de/hw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8192de/hw.c
> index 743ac6871bf4..c336d4b362f5 100644
> --- a/drivers/net/wireless/realtek/rtlwifi/rtl8192de/hw.c
> +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192de/hw.c
> @@ -1684,6 +1684,7 @@ static void _rtl92de_efuse_update_chip_version(struct ieee80211_hw *hw)
>                 rtl_dbg(rtlpriv, COMP_INIT, DBG_LOUD, "D-CUT!!!\n");
>                 break;
>         case 0xCC33:
> +       case 0x33CC:
>                 chipver |= CHIP_92D_E_CUT;
>                 rtl_dbg(rtlpriv, COMP_INIT, DBG_LOUD, "E-CUT!!!\n");
>                 break;
> 
> How did you find this weird value? Vendor driver doesn't work for you?
> 

It works fine. I just saw the message "Unkown CUT!" and got curious:
https://github.com/lwfinger/rtl8192du/issues/92#issuecomment-1155420291

If the addresses in the efuse are correct, then my device doesn't need
any patch. Unknown cut is treated the same as D cut, which is treated
the same as E cut.

> Ping-Ke
>
Ping-Ke Shih Jan. 17, 2024, 12:15 a.m. UTC | #6
> -----Original Message-----
> From: Bitterblue Smith <rtl8821cerfe2@gmail.com>
> Sent: Tuesday, January 16, 2024 11:38 PM
> To: Ping-Ke Shih <pkshih@realtek.com>; linux-wireless@vger.kernel.org
> Cc: Larry Finger <Larry.Finger@lwfinger.net>
> Subject: Re: [PATCH] wifi: rtlwifi: rtl8192de: Fix byte order of chip version
> 
> It works fine. I just saw the message "Unkown CUT!" and got curious:
> https://github.com/lwfinger/rtl8192du/issues/92#issuecomment-1155420291
> 
> If the addresses in the efuse are correct, then my device doesn't need
> any patch. Unknown cut is treated the same as D cut, which is treated
> the same as E cut.
> 

Let's keep it as it was.
diff mbox series

Patch

diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192de/reg.h b/drivers/net/wireless/realtek/rtlwifi/rtl8192de/reg.h
index 2783d7e7b227..bf15c636b092 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8192de/reg.h
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192de/reg.h
@@ -657,8 +657,8 @@ 
 #define EEPROM_RF_OPT7				0xCC
 
 #define EEPROM_DEF_PART_NO			0x3FD    /* Byte */
-#define EEPROME_CHIP_VERSION_L			0x3FF
-#define EEPROME_CHIP_VERSION_H			0x3FE
+#define EEPROME_CHIP_VERSION_L			0x3FE
+#define EEPROME_CHIP_VERSION_H			0x3FF
 
 /*
  * Current IOREG MAP