On my RTW8821CU chipset rfe_option reads as 0x22. Looking at the downstream driver suggests that the field width of rfe_option is 5 bit, so rfe_option should be masked with 0x1f.
Without this the rfe_option comparisons with 2 further down the driver evaluate as false when they should really evaluate as true. The effect is that 2G channels do not work.
rfe_option is also used as an array index into rtw8821c_rfe_defs[]. rtw8821c_rfe_defs[34] (0x22) was added as part of adding USB support, likely because rfe_option reads as 0x22. As this now becomes 0x2, rtw8821c_rfe_defs[34] is no longer used and can be removed.
Signed-off-by: Sascha Hauer s.hauer@pengutronix.de Tested-by: ValdikSS iam@valdikss.org.ru Tested-by: Alexandru gagniuc mr.nuke.me@gmail.com Tested-by: Larry Finger Larry.Finger@lwfinger.net Cc: stable@vger.kernel.org --- drivers/net/wireless/realtek/rtw88/rtw8821c.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtw88/rtw8821c.c b/drivers/net/wireless/realtek/rtw88/rtw8821c.c index 17f800f6efbd0..67efa58dd78ee 100644 --- a/drivers/net/wireless/realtek/rtw88/rtw8821c.c +++ b/drivers/net/wireless/realtek/rtw88/rtw8821c.c @@ -47,7 +47,7 @@ static int rtw8821c_read_efuse(struct rtw_dev *rtwdev, u8 *log_map)
map = (struct rtw8821c_efuse *)log_map;
- efuse->rfe_option = map->rfe_option; + efuse->rfe_option = map->rfe_option & 0x1f; efuse->rf_board_option = map->rf_board_option; efuse->crystal_cap = map->xtal_k; efuse->pa_type_2g = map->pa_type; @@ -1537,7 +1537,6 @@ static const struct rtw_rfe_def rtw8821c_rfe_defs[] = { [2] = RTW_DEF_RFE_EXT(8821c, 0, 0, 2), [4] = RTW_DEF_RFE_EXT(8821c, 0, 0, 2), [6] = RTW_DEF_RFE(8821c, 0, 0), - [34] = RTW_DEF_RFE(8821c, 0, 0), };
static struct rtw_hw_reg rtw8821c_dig[] = {
-----Original Message----- From: Sascha Hauer s.hauer@pengutronix.de Sent: Tuesday, April 4, 2023 3:25 PM To: linux-wireless linux-wireless@vger.kernel.org Cc: Hans Ulli Kroll linux@ulli-kroll.de; Larry Finger Larry.Finger@lwfinger.net; Ping-Ke Shih pkshih@realtek.com; Tim K tpkuester@gmail.com; Alex G . mr.nuke.me@gmail.com; Nick Morrow morrownr@gmail.com; Viktor Petrenko g0000ga@gmail.com; Andreas Henriksson andreas@fatal.se; ValdikSS iam@valdikss.org.ru; kernel@pengutronix.de; Sascha Hauer s.hauer@pengutronix.de; stable@vger.kernel.org Subject: [PATCH v2 2/2] wifi: rtw88: rtw8821c: Fix rfe_option field width
On my RTW8821CU chipset rfe_option reads as 0x22. Looking at the downstream driver suggests that the field width of rfe_option is 5 bit, so rfe_option should be masked with 0x1f.
I don't aware of this. Could you point where you get it?
As I check it internally, 0x22 is expected, so I suggest to have 0x22 entry as below
- [34] = RTW_DEF_RFE(8821c, 0, 0), + [34] = RTW_DEF_RFE_EXT(8821c, 0, 0, 2), // copy from type 2
Without this the rfe_option comparisons with 2 further down the driver evaluate as false when they should really evaluate as true. The effect is that 2G channels do not work.
rfe_option is also used as an array index into rtw8821c_rfe_defs[]. rtw8821c_rfe_defs[34] (0x22) was added as part of adding USB support, likely because rfe_option reads as 0x22. As this now becomes 0x2, rtw8821c_rfe_defs[34] is no longer used and can be removed.
Signed-off-by: Sascha Hauer s.hauer@pengutronix.de Tested-by: ValdikSS iam@valdikss.org.ru Tested-by: Alexandru gagniuc mr.nuke.me@gmail.com Tested-by: Larry Finger Larry.Finger@lwfinger.net Cc: stable@vger.kernel.org
drivers/net/wireless/realtek/rtw88/rtw8821c.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtw88/rtw8821c.c b/drivers/net/wireless/realtek/rtw88/rtw8821c.c index 17f800f6efbd0..67efa58dd78ee 100644 --- a/drivers/net/wireless/realtek/rtw88/rtw8821c.c +++ b/drivers/net/wireless/realtek/rtw88/rtw8821c.c @@ -47,7 +47,7 @@ static int rtw8821c_read_efuse(struct rtw_dev *rtwdev, u8 *log_map)
map = (struct rtw8821c_efuse *)log_map;
efuse->rfe_option = map->rfe_option;
efuse->rfe_option = map->rfe_option & 0x1f; efuse->rf_board_option = map->rf_board_option; efuse->crystal_cap = map->xtal_k; efuse->pa_type_2g = map->pa_type;@@ -1537,7 +1537,6 @@ static const struct rtw_rfe_def rtw8821c_rfe_defs[] = { [2] = RTW_DEF_RFE_EXT(8821c, 0, 0, 2), [4] = RTW_DEF_RFE_EXT(8821c, 0, 0, 2), [6] = RTW_DEF_RFE(8821c, 0, 0),
[34] = RTW_DEF_RFE(8821c, 0, 0),};
static struct rtw_hw_reg rtw8821c_dig[] = {
2.39.2
On Thu, Apr 06, 2023 at 01:54:55AM +0000, Ping-Ke Shih wrote:
-----Original Message----- From: Sascha Hauer s.hauer@pengutronix.de Sent: Tuesday, April 4, 2023 3:25 PM To: linux-wireless linux-wireless@vger.kernel.org Cc: Hans Ulli Kroll linux@ulli-kroll.de; Larry Finger Larry.Finger@lwfinger.net; Ping-Ke Shih pkshih@realtek.com; Tim K tpkuester@gmail.com; Alex G . mr.nuke.me@gmail.com; Nick Morrow morrownr@gmail.com; Viktor Petrenko g0000ga@gmail.com; Andreas Henriksson andreas@fatal.se; ValdikSS iam@valdikss.org.ru; kernel@pengutronix.de; Sascha Hauer s.hauer@pengutronix.de; stable@vger.kernel.org Subject: [PATCH v2 2/2] wifi: rtw88: rtw8821c: Fix rfe_option field width
On my RTW8821CU chipset rfe_option reads as 0x22. Looking at the downstream driver suggests that the field width of rfe_option is 5 bit, so rfe_option should be masked with 0x1f.
I don't aware of this. Could you point where you get it?
See https://github.com/morrownr/8821cu-20210916/blob/main/hal/btc/halbtc8821c1an... and https://github.com/morrownr/8821cu-20210916/blob/main/hal/btc/halbtc8821c2an...
But I now see that this masked value is only used at the places I pointed to, there are other places in the driver that use the unmasked value.
As I check it internally, 0x22 is expected, so I suggest to have 0x22 entry as below
[34] = RTW_DEF_RFE(8821c, 0, 0),
[34] = RTW_DEF_RFE_EXT(8821c, 0, 0, 2), // copy from type 2
That alone is not enough. There are other places in rtw8821c.c that compare with rfe_option. See below for a patch with annotations where to find the corresponding code in the downstream driver. Note how BIT(5) is irrelevant for all decisions. I can't tell of course if that's just by chance or by intent.
I don't know where to go from here. It looks like we really only want to make a decision between SWITCH_TO_WLG and SWITCH_TO_BTG at most places, so it might be better to store a flag somewhere rather than having the big switch/case in multiple places.
Sascha
-------------------------------8<---------------------------------
diff --git a/drivers/net/wireless/realtek/rtw88/rtw8821c.c b/drivers/net/wireless/realtek/rtw88/rtw8821c.c index 17f800f6efbd0..5da7787cea129 100644 --- a/drivers/net/wireless/realtek/rtw88/rtw8821c.c +++ b/drivers/net/wireless/realtek/rtw88/rtw8821c.c @@ -317,11 +317,32 @@ static void rtw8821c_set_channel_rf(struct rtw_dev *rtwdev, u8 channel, u8 bw) }
if (channel <= 14) { - if (rtwdev->efuse.rfe_option == 0) - rtw8821c_switch_rf_set(rtwdev, SWITCH_TO_WLG); - else if (rtwdev->efuse.rfe_option == 2 || - rtwdev->efuse.rfe_option == 4) + /* + * see: + * https://github.com/morrownr/8821cu-20210916/blob/main/hal/phydm/rtl8821c/phy... + */ + switch (rtwdev->efuse.rfe_option) { + case 0x02: case 0x22: + case 0x04: case 0x24: + case 0x07: case 0x27: + case 0x2a: + case 0x2c: + case 0x2f: rtw8821c_switch_rf_set(rtwdev, SWITCH_TO_BTG); + break; + case 0x00: case 0x20: + case 0x01: case 0x21: + case 0x03: case 0x23: + case 0x05: case 0x25: + case 0x06: case 0x26: + case 0x28: + case 0x29: + case 0x2b: + case 0x2d: + case 0x2e: + default: + rtw8821c_switch_rf_set(rtwdev, SWITCH_TO_WLG); + } rtw_write_rf(rtwdev, RF_PATH_A, RF_LUTDBG, BIT(6), 0x1); rtw_write_rf(rtwdev, RF_PATH_A, 0x64, 0xf, 0xf); } else { @@ -501,12 +522,35 @@ static s8 get_cck_rx_pwr(struct rtw_dev *rtwdev, u8 lna_idx, u8 vga_idx) s8 rx_pwr_all = 0; s8 lna_gain = 0;
- if (efuse->rfe_option == 0) { - lna_gain_table = lna_gain_table_0; - lna_gain_table_size = ARRAY_SIZE(lna_gain_table_0); - } else { + /* + * see: + * https://github.com/morrownr/8821cu-20210916/blob/main/hal/phydm/rtl8821c/phy... + * https://github.com/morrownr/8821cu-20210916/blob/main/hal/phydm/phydm.c#L178 + */ + switch (rtwdev->efuse.rfe_option) { + case 0x02: case 0x22: + case 0x04: case 0x24: + case 0x07: case 0x27: + case 0x2a: + case 0x2c: + case 0x2f: lna_gain_table = lna_gain_table_1; lna_gain_table_size = ARRAY_SIZE(lna_gain_table_1); + break; + case 0x00: case 0x20: + case 0x01: case 0x21: + case 0x03: case 0x23: + case 0x05: case 0x25: + case 0x06: case 0x26: + case 0x28: + case 0x29: + case 0x2b: + case 0x2d: + case 0x2e: + default: + lna_gain_table = lna_gain_table_0; + lna_gain_table_size = ARRAY_SIZE(lna_gain_table_0); + break; }
if (lna_idx >= lna_gain_table_size) { @@ -821,6 +865,9 @@ static void rtw8821c_coex_cfg_ant_switch(struct rtw_dev *rtwdev, u8 ctrl_type, DPDT_CTRL_PIN);
if (pos_type == COEX_SWITCH_TO_WLG_BT) { + /* + * What here? Cannot find refval = 0x3 in downstream driver + */ if (coex_rfe->rfe_module_type != 0x4 && coex_rfe->rfe_module_type != 0x2) regval = 0x3; @@ -902,7 +949,12 @@ static void rtw8821c_coex_cfg_rfe_type(struct rtw_dev *rtwdev) coex_rfe->ant_switch_exist = true; coex_rfe->wlg_at_btg = false;
- switch (coex_rfe->rfe_module_type) { + /* + * see: + * https://github.com/morrownr/8821cu-20210916/blob/main/hal/btc/halbtc8821c1an... + * https://github.com/morrownr/8821cu-20210916/blob/main/hal/btc/halbtc8821c2an... + */ + switch (coex_rfe->rfe_module_type & 0x1f) { case 0: case 8: case 1: @@ -1533,11 +1585,30 @@ static const struct rtw_intf_phy_para_table phy_para_table_8821c = { };
static const struct rtw_rfe_def rtw8821c_rfe_defs[] = { - [0] = RTW_DEF_RFE(8821c, 0, 0), - [2] = RTW_DEF_RFE_EXT(8821c, 0, 0, 2), - [4] = RTW_DEF_RFE_EXT(8821c, 0, 0, 2), - [6] = RTW_DEF_RFE(8821c, 0, 0), - [34] = RTW_DEF_RFE(8821c, 0, 0), + [0x00] = RTW_DEF_RFE(8821c, 0, 0), + [0x01] = RTW_DEF_RFE(8821c, 0, 0), + [0x02] = RTW_DEF_RFE_EXT(8821c, 0, 0, 2), + [0x03] = RTW_DEF_RFE(8821c, 0, 0), + [0x04] = RTW_DEF_RFE_EXT(8821c, 0, 0, 2), + [0x05] = RTW_DEF_RFE(8821c, 0, 0), + [0x06] = RTW_DEF_RFE(8821c, 0, 0), + [0x07] = RTW_DEF_RFE_EXT(8821c, 0, 0, 2), + [0x20] = RTW_DEF_RFE(8821c, 0, 0), + [0x21] = RTW_DEF_RFE(8821c, 0, 0), + [0x22] = RTW_DEF_RFE_EXT(8821c, 0, 0, 2), + [0x23] = RTW_DEF_RFE(8821c, 0, 0), + [0x24] = RTW_DEF_RFE_EXT(8821c, 0, 0, 2), + [0x25] = RTW_DEF_RFE(8821c, 0, 0), + [0x26] = RTW_DEF_RFE(8821c, 0, 0), + [0x27] = RTW_DEF_RFE_EXT(8821c, 0, 0, 2), + [0x28] = RTW_DEF_RFE(8821c, 0, 0), + [0x29] = RTW_DEF_RFE(8821c, 0, 0), + [0x2a] = RTW_DEF_RFE_EXT(8821c, 0, 0, 2), + [0x2b] = RTW_DEF_RFE(8821c, 0, 0), + [0x2c] = RTW_DEF_RFE_EXT(8821c, 0, 0, 2), + [0x2d] = RTW_DEF_RFE(8821c, 0, 0), + [0x2e] = RTW_DEF_RFE(8821c, 0, 0), + [0x2f] = RTW_DEF_RFE_EXT(8821c, 0, 0, 2), };
static struct rtw_hw_reg rtw8821c_dig[] = {
-----Original Message----- From: Sascha Hauer s.hauer@pengutronix.de Sent: Tuesday, April 11, 2023 6:26 PM To: Ping-Ke Shih pkshih@realtek.com Cc: linux-wireless linux-wireless@vger.kernel.org; Hans Ulli Kroll linux@ulli-kroll.de; Larry Finger Larry.Finger@lwfinger.net; Tim K tpkuester@gmail.com; Alex G . mr.nuke.me@gmail.com; Nick Morrow morrownr@gmail.com; Viktor Petrenko g0000ga@gmail.com; Andreas Henriksson andreas@fatal.se; ValdikSS iam@valdikss.org.ru; kernel@pengutronix.de; stable@vger.kernel.org Subject: Re: [PATCH v2 2/2] wifi: rtw88: rtw8821c: Fix rfe_option field width
On Thu, Apr 06, 2023 at 01:54:55AM +0000, Ping-Ke Shih wrote:
-----Original Message----- From: Sascha Hauer s.hauer@pengutronix.de Sent: Tuesday, April 4, 2023 3:25 PM To: linux-wireless linux-wireless@vger.kernel.org Cc: Hans Ulli Kroll linux@ulli-kroll.de; Larry Finger Larry.Finger@lwfinger.net; Ping-Ke Shih pkshih@realtek.com; Tim K tpkuester@gmail.com; Alex G . mr.nuke.me@gmail.com; Nick Morrow morrownr@gmail.com; Viktor Petrenko g0000ga@gmail.com; Andreas Henriksson andreas@fatal.se; ValdikSS iam@valdikss.org.ru; kernel@pengutronix.de; Sascha Hauer s.hauer@pengutronix.de; stable@vger.kernel.org Subject: [PATCH v2 2/2] wifi: rtw88: rtw8821c: Fix rfe_option field width
On my RTW8821CU chipset rfe_option reads as 0x22. Looking at the downstream driver suggests that the field width of rfe_option is 5 bit, so rfe_option should be masked with 0x1f.
I don't aware of this. Could you point where you get it?
See https://github.com/morrownr/8821cu-20210916/blob/main/hal/btc/halbtc8821c1an... and https://github.com/morrownr/8821cu-20210916/blob/main/hal/btc/halbtc8821c2an...
But I now see that this masked value is only used at the places I pointed to, there are other places in the driver that use the unmasked value.
After I read vendor driver, there are three variety of rfe_option for 8821C. 1. raw value from efuse hal->rfe_type = map[EEPROM_RFE_OPTION_8821C];
2. BT-coexistence rfe_type->rfe_module_type = board_info->rfe_type & 0x1f;
3. PHY dm->rfe_type_expand = hal->rfe_type = raw value dm->rfe_type = dm->rfe_type_expand >> 3;
For rtw88, there are only two variety, but they are identical coex_rfe->rfe_module_type = efuse->rfe_option;
The flaws are rfe_type->rfe_module_type of item 2 and dm->rfe_type of item 3 above, and most things are addressed by your draft patch. Exception is check_positive() check dm->rfe_type, but we don't have this conversion in rtw88 (i.e. cond.rfe = efuse->rfe_option; in rtw_phy_setup_phy_cond()).
Since I don't have a hardware with rfe_option larger than 8, could you please give below patch a try?
--- a/phy.c +++ b/phy.c @@ -1048,6 +1048,9 @@ void rtw_phy_setup_phy_cond(struct rtw_dev *rtwdev, u32 pkg) cond.plat = 0x04; cond.rfe = efuse->rfe_option;
+ if (rtwdev->chip->id == RTW_CHIP_TYPE_8821C) + cond.rfe = efuse->rfe_option >> 3; + switch (rtw_hci_type(rtwdev)) { case RTW_HCI_TYPE_USB: cond.intf = INTF_USB;
8821C is more complex than others, and I'm not familiar with it, so maybe I could miss something. Please correct me if any.
As I check it internally, 0x22 is expected, so I suggest to have 0x22 entry as below
[34] = RTW_DEF_RFE(8821c, 0, 0),
[34] = RTW_DEF_RFE_EXT(8821c, 0, 0, 2), // copy from type 2That alone is not enough. There are other places in rtw8821c.c that compare with rfe_option. See below for a patch with annotations where to find the corresponding code in the downstream driver. Note how BIT(5) is irrelevant for all decisions. I can't tell of course if that's just by chance or by intent.
You're right. I miss these points.
I don't know where to go from here. It looks like we really only want to make a decision between SWITCH_TO_WLG and SWITCH_TO_BTG at most places, so it might be better to store a flag somewhere rather than having the big switch/case in multiple places.
Agreed. Add something like:
--- a/main.h +++ b/main.h @@ -2076,6 +2076,7 @@ struct rtw_hal { u8 mp_chip; u8 oem_id; struct rtw_phy_cond phy_cond; + bool rfe_btg;
u8 ps_mode; u8 current_channel;
--- a/rtw8821c.c +++ b/rtw8821c.c @@ -47,6 +47,7 @@ enum rtw8821ce_rf_set {
static int rtw8821c_read_efuse(struct rtw_dev *rtwdev, u8 *log_map) { + struct rtw_hal *hal = &rtwdev->hal; struct rtw_efuse *efuse = &rtwdev->efuse; struct rtw8821c_efuse *map; int i; @@ -91,6 +92,12 @@ static int rtw8821c_read_efuse(struct rtw_dev *rtwdev, u8 *log_map) return -ENOTSUPP; }
+ switch (efuse->rfe_option) { + case 0x02: case 0x22: // ... + hal->rfe_btg = true; + break; + } + return 0; }
[...]
static const struct rtw_rfe_def rtw8821c_rfe_defs[] = {
[0] = RTW_DEF_RFE(8821c, 0, 0),[2] = RTW_DEF_RFE_EXT(8821c, 0, 0, 2),[4] = RTW_DEF_RFE_EXT(8821c, 0, 0, 2),[6] = RTW_DEF_RFE(8821c, 0, 0),[34] = RTW_DEF_RFE(8821c, 0, 0),
[0x00] = RTW_DEF_RFE(8821c, 0, 0),[0x01] = RTW_DEF_RFE(8821c, 0, 0),[0x02] = RTW_DEF_RFE_EXT(8821c, 0, 0, 2),[0x03] = RTW_DEF_RFE(8821c, 0, 0),[0x04] = RTW_DEF_RFE_EXT(8821c, 0, 0, 2),[0x05] = RTW_DEF_RFE(8821c, 0, 0),[0x06] = RTW_DEF_RFE(8821c, 0, 0),[0x07] = RTW_DEF_RFE_EXT(8821c, 0, 0, 2),[0x20] = RTW_DEF_RFE(8821c, 0, 0),[0x21] = RTW_DEF_RFE(8821c, 0, 0),[0x22] = RTW_DEF_RFE_EXT(8821c, 0, 0, 2),[0x23] = RTW_DEF_RFE(8821c, 0, 0),[0x24] = RTW_DEF_RFE_EXT(8821c, 0, 0, 2),[0x25] = RTW_DEF_RFE(8821c, 0, 0),[0x26] = RTW_DEF_RFE(8821c, 0, 0),[0x27] = RTW_DEF_RFE_EXT(8821c, 0, 0, 2),[0x28] = RTW_DEF_RFE(8821c, 0, 0),[0x29] = RTW_DEF_RFE(8821c, 0, 0),[0x2a] = RTW_DEF_RFE_EXT(8821c, 0, 0, 2),[0x2b] = RTW_DEF_RFE(8821c, 0, 0),[0x2c] = RTW_DEF_RFE_EXT(8821c, 0, 0, 2),[0x2d] = RTW_DEF_RFE(8821c, 0, 0),[0x2e] = RTW_DEF_RFE(8821c, 0, 0),[0x2f] = RTW_DEF_RFE_EXT(8821c, 0, 0, 2),
I'm not sure if we add all of them, since some aren't tested, but maybe it would be better than nothing.
Ping-Ke
On Fri, Apr 14, 2023 at 02:05:44AM +0000, Ping-Ke Shih wrote:
-----Original Message----- From: Sascha Hauer s.hauer@pengutronix.de Sent: Tuesday, April 11, 2023 6:26 PM To: Ping-Ke Shih pkshih@realtek.com Cc: linux-wireless linux-wireless@vger.kernel.org; Hans Ulli Kroll linux@ulli-kroll.de; Larry Finger Larry.Finger@lwfinger.net; Tim K tpkuester@gmail.com; Alex G . mr.nuke.me@gmail.com; Nick Morrow morrownr@gmail.com; Viktor Petrenko g0000ga@gmail.com; Andreas Henriksson andreas@fatal.se; ValdikSS iam@valdikss.org.ru; kernel@pengutronix.de; stable@vger.kernel.org Subject: Re: [PATCH v2 2/2] wifi: rtw88: rtw8821c: Fix rfe_option field width
On Thu, Apr 06, 2023 at 01:54:55AM +0000, Ping-Ke Shih wrote:
-----Original Message----- From: Sascha Hauer s.hauer@pengutronix.de Sent: Tuesday, April 4, 2023 3:25 PM To: linux-wireless linux-wireless@vger.kernel.org Cc: Hans Ulli Kroll linux@ulli-kroll.de; Larry Finger Larry.Finger@lwfinger.net; Ping-Ke Shih pkshih@realtek.com; Tim K tpkuester@gmail.com; Alex G . mr.nuke.me@gmail.com; Nick Morrow morrownr@gmail.com; Viktor Petrenko g0000ga@gmail.com; Andreas Henriksson andreas@fatal.se; ValdikSS iam@valdikss.org.ru; kernel@pengutronix.de; Sascha Hauer s.hauer@pengutronix.de; stable@vger.kernel.org Subject: [PATCH v2 2/2] wifi: rtw88: rtw8821c: Fix rfe_option field width
On my RTW8821CU chipset rfe_option reads as 0x22. Looking at the downstream driver suggests that the field width of rfe_option is 5 bit, so rfe_option should be masked with 0x1f.
I don't aware of this. Could you point where you get it?
See https://github.com/morrownr/8821cu-20210916/blob/main/hal/btc/halbtc8821c1an... and https://github.com/morrownr/8821cu-20210916/blob/main/hal/btc/halbtc8821c2an...
But I now see that this masked value is only used at the places I pointed to, there are other places in the driver that use the unmasked value.
After I read vendor driver, there are three variety of rfe_option for 8821C.
raw value from efuse hal->rfe_type = map[EEPROM_RFE_OPTION_8821C];
BT-coexistence rfe_type->rfe_module_type = board_info->rfe_type & 0x1f;
PHY dm->rfe_type_expand = hal->rfe_type = raw value dm->rfe_type = dm->rfe_type_expand >> 3;
For rtw88, there are only two variety, but they are identical coex_rfe->rfe_module_type = efuse->rfe_option;
The flaws are rfe_type->rfe_module_type of item 2 and dm->rfe_type of item 3 above, and most things are addressed by your draft patch. Exception is check_positive() check dm->rfe_type, but we don't have this conversion in rtw88 (i.e. cond.rfe = efuse->rfe_option; in rtw_phy_setup_phy_cond()).
Since I don't have a hardware with rfe_option larger than 8, could you please give below patch a try?
--- a/phy.c +++ b/phy.c @@ -1048,6 +1048,9 @@ void rtw_phy_setup_phy_cond(struct rtw_dev *rtwdev, u32 pkg) cond.plat = 0x04; cond.rfe = efuse->rfe_option;
if (rtwdev->chip->id == RTW_CHIP_TYPE_8821C)cond.rfe = efuse->rfe_option >> 3;switch (rtw_hci_type(rtwdev)) { case RTW_HCI_TYPE_USB: cond.intf = INTF_USB;
This change doesn't make any difference. cond.rfe is only used in check_positive() which is implemented like this:
static bool check_positive(struct rtw_dev *rtwdev, struct rtw_phy_cond cond) { struct rtw_hal *hal = &rtwdev->hal; struct rtw_phy_cond drv_cond = hal->phy_cond;
if (cond.cut && cond.cut != drv_cond.cut) return false;
if (cond.pkg && cond.pkg != drv_cond.pkg) return false;
if (cond.intf && cond.intf != drv_cond.intf) return false;
if (cond.rfe != drv_cond.rfe) return false;
return true; }
In my case check_positive() always returns early when comparing cond.pkg which is always set to '15' in rtw_phy_setup_phy_cond():
void rtw_phy_setup_phy_cond(struct rtw_dev *rtwdev, u32 pkg) { ... cond.pkg = pkg ? pkg : 15; ... }
In the upstream driver rtw_phy_setup_phy_cond() is only ever called with '0' as pkg argument. Now in the downstream driver I found this snippet:
void phydm_init_hw_info_by_rfe_type_8821c(struct dm_struct *dm) { ...
if (dm->rfe_type_expand == 2 || dm->rfe_type_expand == 4 || dm->rfe_type_expand == 7) { dm->default_rf_set_8821c = SWITCH_TO_BTG; } else if (dm->rfe_type_expand == 0 || dm->rfe_type_expand == 1 || dm->rfe_type_expand == 3 || dm->rfe_type_expand == 5 || dm->rfe_type_expand == 6) { dm->default_rf_set_8821c = SWITCH_TO_WLG; } else if (dm->rfe_type_expand == 0x22 || dm->rfe_type_expand == 0x24 || dm->rfe_type_expand == 0x27 || dm->rfe_type_expand == 0x2a || dm->rfe_type_expand == 0x2c || dm->rfe_type_expand == 0x2f) { dm->default_rf_set_8821c = SWITCH_TO_BTG; odm_cmn_info_init(dm, ODM_CMNINFO_PACKAGE_TYPE, 1); } else if (dm->rfe_type_expand == 0x20 || dm->rfe_type_expand == 0x21 || dm->rfe_type_expand == 0x23 || dm->rfe_type_expand == 0x25 || dm->rfe_type_expand == 0x26 || dm->rfe_type_expand == 0x28 || dm->rfe_type_expand == 0x29 || dm->rfe_type_expand == 0x2b || dm->rfe_type_expand == 0x2d || dm->rfe_type_expand == 0x2e) { dm->default_rf_set_8821c = SWITCH_TO_WLG; odm_cmn_info_init(dm, ODM_CMNINFO_PACKAGE_TYPE, 1); }
... }
odm_cmn_info_init(dm, ODM_CMNINFO_PACKAGE_TYPE, 1); seems to be the analogue of the pkg type argument. This suggests that for rfe_type >= 0x20 we should call rtw_phy_setup_phy_cond() with '1' as pkg argument. When doing this here I will end up with check_positive() returning 'true' in some cases. However, I didn't notice any change in the driver behaviour then.
Note how the above code snippet looks like the rfe_type is indeed encoded in the lower 5 bits whereas BIT(5) could be used as the package type. That could be by chance, but it's striking that rfe_type (x) always ends up in the same code path as (x + 0x20) also in other parts of this file.
I'm a bit lost here. I suggest that we stick with the variant I tried in v1 of this series, but I'll add a note that there might be some inaccuracies in how some currently unknown chip variants are handled.
Sascha
linux-stable-mirror@lists.linaro.org
-
Ping-Ke Shih -
Sascha Hauer