| Message ID | 20230417140358.2240429-4-s.hauer@pengutronix.de |
|---|---|
| State | New |
| Headers | show |
| Series | RTW88 USB bug fixes | expand |
> -----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> [..]
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<--------------------------------
> -----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
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 --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];
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(-)