diff mbox series

[v3,3/4] wifi: rtw88: set pkg_type correctly for specific rtw8821c variants

Message ID 20230417140358.2240429-4-s.hauer@pengutronix.de (mailing list archive)
State Accepted
Commit 97c75e1adeda78b3794936c617d8b86e9ebd54f5
Delegated to: Kalle Valo
Headers show
Series RTW88 USB bug fixes | expand

Commit Message

Sascha Hauer April 17, 2023, 2:03 p.m. UTC
According to the vendor driver the pkg_type has to be set to '1'
for some rtw8821c variants. As the pkg_type has been hardcoded to
'0', add a field for it in struct rtw_hal and set this correctly
in the rtw8821c part.
With this parsing of a rtw_table is influenced and check_positive()
in phy.c returns true for some cases here. The same is done in the
vendor driver. However, this has no visible effect on the driver
here.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/net/wireless/realtek/rtw88/main.c     | 2 +-
 drivers/net/wireless/realtek/rtw88/main.h     | 1 +
 drivers/net/wireless/realtek/rtw88/rtw8821c.c | 3 +++
 3 files changed, 5 insertions(+), 1 deletion(-)

Comments

Ping-Ke Shih April 18, 2023, 12:36 a.m. UTC | #1
> -----Original Message-----
> From: Sascha Hauer <s.hauer@pengutronix.de>
> Sent: Monday, April 17, 2023 10:04 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>
> Subject: [PATCH v3 3/4] wifi: rtw88: set pkg_type correctly for specific rtw8821c variants
> 
> According to the vendor driver the pkg_type has to be set to '1'
> for some rtw8821c variants. As the pkg_type has been hardcoded to
> '0', add a field for it in struct rtw_hal and set this correctly
> in the rtw8821c part.
> With this parsing of a rtw_table is influenced and check_positive()
> in phy.c returns true for some cases here. The same is done in the
> vendor driver. However, this has no visible effect on the driver
> here.

I agree this patch, but still want to know more about the meaning of
"...no visible effect...". Do you mean your USB device works well with/without
this patch? or, IO is absolutely the same when loading parameters with
check_positive()?


> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>

[..]
Sascha Hauer April 18, 2023, 8:58 a.m. UTC | #2
On Tue, Apr 18, 2023 at 12:36:31AM +0000, Ping-Ke Shih wrote:
> 
> 
> > -----Original Message-----
> > From: Sascha Hauer <s.hauer@pengutronix.de>
> > Sent: Monday, April 17, 2023 10:04 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>
> > Subject: [PATCH v3 3/4] wifi: rtw88: set pkg_type correctly for specific rtw8821c variants
> > 
> > According to the vendor driver the pkg_type has to be set to '1'
> > for some rtw8821c variants. As the pkg_type has been hardcoded to
> > '0', add a field for it in struct rtw_hal and set this correctly
> > in the rtw8821c part.
> > With this parsing of a rtw_table is influenced and check_positive()
> > in phy.c returns true for some cases here. The same is done in the
> > vendor driver. However, this has no visible effect on the driver
> > here.
> 
> I agree this patch, but still want to know more about the meaning of
> "...no visible effect...". Do you mean your USB device works well with/without
> this patch? or, IO is absolutely the same when loading parameters with
> check_positive()?

Yes, it works with and without this patch. With this patch
check_positive() returns true in some cases whereas without this patch
check_positive always returns false.
I don't know at all what effect this change could have, maybe I just
need the right test case to verify it really makes a change.

I just realized that something like the below is missing, as the
cond.rfe part needs the raw rfe value from fuses >> 3.

Maybe we just take 1/4 and 2/4 and drop the others. I am running out of
time for further debugging RTW8821C which is a chip our customer isn't
interested in.

Sascha


-------------------------------8<--------------------------------

From 70e0bbf1e1d3949dede9f814d39970e5b27d3329 Mon Sep 17 00:00:00 2001
From: Sascha Hauer <s.hauer@pengutronix.de>
Date: Tue, 18 Apr 2023 10:33:56 +0200
Subject: [PATCH] wifi: rtw88: rtw8821c: set rfe correctly in phy code

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/net/wireless/realtek/rtw88/main.h     | 1 +
 drivers/net/wireless/realtek/rtw88/phy.c      | 3 +++
 drivers/net/wireless/realtek/rtw88/rtw8821c.c | 1 +
 3 files changed, 5 insertions(+)

diff --git a/drivers/net/wireless/realtek/rtw88/main.h b/drivers/net/wireless/realtek/rtw88/main.h
index 462f69547be03..a75c86981acf7 100644
--- a/drivers/net/wireless/realtek/rtw88/main.h
+++ b/drivers/net/wireless/realtek/rtw88/main.h
@@ -1738,6 +1738,7 @@ struct rtw_efuse {
 	u8 country_code[2];
 	u8 rf_board_option;
 	u8 rfe_option;
+	u8 rfe_type;
 	u8 power_track_type;
 	u8 thermal_meter[RTW_RF_PATH_MAX];
 	u8 thermal_meter_k;
diff --git a/drivers/net/wireless/realtek/rtw88/phy.c b/drivers/net/wireless/realtek/rtw88/phy.c
index 128e75a81bf3c..deb39cbea440f 100644
--- a/drivers/net/wireless/realtek/rtw88/phy.c
+++ b/drivers/net/wireless/realtek/rtw88/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_type;
+
 	switch (rtw_hci_type(rtwdev)) {
 	case RTW_HCI_TYPE_USB:
 		cond.intf = INTF_USB;
diff --git a/drivers/net/wireless/realtek/rtw88/rtw8821c.c b/drivers/net/wireless/realtek/rtw88/rtw8821c.c
index a50753ae235b5..72485c9471f11 100644
--- a/drivers/net/wireless/realtek/rtw88/rtw8821c.c
+++ b/drivers/net/wireless/realtek/rtw88/rtw8821c.c
@@ -49,6 +49,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 & 0x1f;
+	efuse->rfe_type = map->rfe_option >> 3;
 	efuse->rf_board_option = map->rf_board_option;
 	efuse->crystal_cap = map->xtal_k;
 	efuse->pa_type_2g = map->pa_type;
Ping-Ke Shih April 19, 2023, 12:20 a.m. UTC | #3
> -----Original Message-----
> From: Sascha Hauer <s.hauer@pengutronix.de>
> Sent: Tuesday, April 18, 2023 4:58 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
> Subject: Re: [PATCH v3 3/4] wifi: rtw88: set pkg_type correctly for specific rtw8821c variants
> 
> On Tue, Apr 18, 2023 at 12:36:31AM +0000, Ping-Ke Shih wrote:
> >
> >
> > > -----Original Message-----
> > > From: Sascha Hauer <s.hauer@pengutronix.de>
> > > Sent: Monday, April 17, 2023 10:04 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>
> > > Subject: [PATCH v3 3/4] wifi: rtw88: set pkg_type correctly for specific rtw8821c variants
> > >
> > > According to the vendor driver the pkg_type has to be set to '1'
> > > for some rtw8821c variants. As the pkg_type has been hardcoded to
> > > '0', add a field for it in struct rtw_hal and set this correctly
> > > in the rtw8821c part.
> > > With this parsing of a rtw_table is influenced and check_positive()
> > > in phy.c returns true for some cases here. The same is done in the
> > > vendor driver. However, this has no visible effect on the driver
> > > here.
> >
> > I agree this patch, but still want to know more about the meaning of
> > "...no visible effect...". Do you mean your USB device works well with/without
> > this patch? or, IO is absolutely the same when loading parameters with
> > check_positive()?
> 
> Yes, it works with and without this patch. With this patch
> check_positive() returns true in some cases whereas without this patch
> check_positive always returns false.
> I don't know at all what effect this change could have, maybe I just
> need the right test case to verify it really makes a change.
> 
> I just realized that something like the below is missing, as the
> cond.rfe part needs the raw rfe value from fuses >> 3.
> 
> Maybe we just take 1/4 and 2/4 and drop the others. I am running out of
> time for further debugging RTW8821C which is a chip our customer isn't
> interested in.
> 

I think we can take all patches, because they go forward to correct direction,
and other flaws can be fixed after people can really get that kind of modules.

Ping-Ke
Sascha Hauer April 19, 2023, 7:21 a.m. UTC | #4
On Wed, Apr 19, 2023 at 12:20:33AM +0000, Ping-Ke Shih wrote:
> 
> 
> > -----Original Message-----
> > From: Sascha Hauer <s.hauer@pengutronix.de>
> > Sent: Tuesday, April 18, 2023 4:58 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
> > Subject: Re: [PATCH v3 3/4] wifi: rtw88: set pkg_type correctly for specific rtw8821c variants
> > 
> > On Tue, Apr 18, 2023 at 12:36:31AM +0000, Ping-Ke Shih wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Sascha Hauer <s.hauer@pengutronix.de>
> > > > Sent: Monday, April 17, 2023 10:04 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>
> > > > Subject: [PATCH v3 3/4] wifi: rtw88: set pkg_type correctly for specific rtw8821c variants
> > > >
> > > > According to the vendor driver the pkg_type has to be set to '1'
> > > > for some rtw8821c variants. As the pkg_type has been hardcoded to
> > > > '0', add a field for it in struct rtw_hal and set this correctly
> > > > in the rtw8821c part.
> > > > With this parsing of a rtw_table is influenced and check_positive()
> > > > in phy.c returns true for some cases here. The same is done in the
> > > > vendor driver. However, this has no visible effect on the driver
> > > > here.
> > >
> > > I agree this patch, but still want to know more about the meaning of
> > > "...no visible effect...". Do you mean your USB device works well with/without
> > > this patch? or, IO is absolutely the same when loading parameters with
> > > check_positive()?
> > 
> > Yes, it works with and without this patch. With this patch
> > check_positive() returns true in some cases whereas without this patch
> > check_positive always returns false.
> > I don't know at all what effect this change could have, maybe I just
> > need the right test case to verify it really makes a change.
> > 
> > I just realized that something like the below is missing, as the
> > cond.rfe part needs the raw rfe value from fuses >> 3.
> > 
> > Maybe we just take 1/4 and 2/4 and drop the others. I am running out of
> > time for further debugging RTW8821C which is a chip our customer isn't
> > interested in.
> > 
> 
> I think we can take all patches, because they go forward to correct direction,
> and other flaws can be fixed after people can really get that kind of modules.

Fine with me, thanks.

Sascha
diff mbox series

Patch

diff --git a/drivers/net/wireless/realtek/rtw88/main.c b/drivers/net/wireless/realtek/rtw88/main.c
index b2e78737bd5d0..421e25353c62f 100644
--- a/drivers/net/wireless/realtek/rtw88/main.c
+++ b/drivers/net/wireless/realtek/rtw88/main.c
@@ -1979,7 +1979,7 @@  static int rtw_chip_board_info_setup(struct rtw_dev *rtwdev)
 	if (!rfe_def)
 		return -ENODEV;
 
-	rtw_phy_setup_phy_cond(rtwdev, 0);
+	rtw_phy_setup_phy_cond(rtwdev, hal->pkg_type);
 
 	rtw_phy_init_tx_power(rtwdev);
 	if (rfe_def->agc_btg_tbl)
diff --git a/drivers/net/wireless/realtek/rtw88/main.h b/drivers/net/wireless/realtek/rtw88/main.h
index d4a53d5567451..9946aca7a72ce 100644
--- a/drivers/net/wireless/realtek/rtw88/main.h
+++ b/drivers/net/wireless/realtek/rtw88/main.h
@@ -1890,6 +1890,7 @@  struct rtw_hal {
 	u8 cut_version;
 	u8 mp_chip;
 	u8 oem_id;
+	u8 pkg_type;
 	struct rtw_phy_cond phy_cond;
 
 	u8 ps_mode;
diff --git a/drivers/net/wireless/realtek/rtw88/rtw8821c.c b/drivers/net/wireless/realtek/rtw88/rtw8821c.c
index 67efa58dd78ee..94c582a27b9ff 100644
--- a/drivers/net/wireless/realtek/rtw88/rtw8821c.c
+++ b/drivers/net/wireless/realtek/rtw88/rtw8821c.c
@@ -41,6 +41,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;
@@ -64,6 +65,8 @@  static int rtw8821c_read_efuse(struct rtw_dev *rtwdev, u8 *log_map)
 	efuse->tx_bb_swing_setting_2g = map->tx_bb_swing_setting_2g;
 	efuse->tx_bb_swing_setting_5g = map->tx_bb_swing_setting_5g;
 
+	hal->pkg_type = map->rfe_option & BIT(5) ? 1 : 0;
+
 	for (i = 0; i < 4; i++)
 		efuse->txpwr_idx_table[i] = map->txpwr_idx_table[i];