| Message ID | 6038bad5-4a4e-4f99-8292-e37a6d11961c@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | wifi: rtw89: Add support for USB devices | expand |
Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote: > A few unnecessary error messages are printed when the device is > unplugged. "read swsi busy" in particular can appear ~1000 times when > RTL8851BU is unplugged. > > Add a new flag RTW89_FLAG_UNPLUGGED and print some error messages only > when this flag is not set. The new USB driver will set the flag when > the device is unplugged. > > Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com> Acked-by: Ping-Ke Shih <pkshih@realtek.com> [...] > diff --git a/drivers/net/wireless/realtek/rtw89/mac.c b/drivers/net/wireless/realtek/rtw89/mac.c > index 1a03355b340f..99f01fff90fe 100644 > --- a/drivers/net/wireless/realtek/rtw89/mac.c > +++ b/drivers/net/wireless/realtek/rtw89/mac.c > @@ -88,7 +88,7 @@ int rtw89_mac_write_lte(struct rtw89_dev *rtwdev, const u32 offset, u32 val) > > ret = read_poll_timeout(rtw89_read8, lte_ctrl, (lte_ctrl & BIT(5)) != 0, > 50, 50000, false, rtwdev, R_AX_LTE_CTRL + 3); For this case, timeout time is large enough for USB. But I'm surprising that you don't need to adjust timeout time of read_poll_timeout() for USB devices, since USB is much slower than PCIE. If sometime you need, I suggest this pattern (number is artificial): u64 rtw89_hci_timeout(rtwdev, to) { if (USB 2) return max(to, 200); // I assume USB 2 is slower and 200 is enough for two times IO. else if (USB 3) return max(to, 100); // I assume USB 3 is faster than USB 2 return to; } u64 to; to = rtw89_hci_timeout(rtwdev, 30); - read_poll_timeout(..., 1, 30, ...); + read_poll_timeout(..., 1, to, ...);
On 13/05/2025 06:43, Ping-Ke Shih wrote: > Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote: >> A few unnecessary error messages are printed when the device is >> unplugged. "read swsi busy" in particular can appear ~1000 times when >> RTL8851BU is unplugged. >> >> Add a new flag RTW89_FLAG_UNPLUGGED and print some error messages only >> when this flag is not set. The new USB driver will set the flag when >> the device is unplugged. >> >> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com> > > Acked-by: Ping-Ke Shih <pkshih@realtek.com> > > [...] > >> diff --git a/drivers/net/wireless/realtek/rtw89/mac.c b/drivers/net/wireless/realtek/rtw89/mac.c >> index 1a03355b340f..99f01fff90fe 100644 >> --- a/drivers/net/wireless/realtek/rtw89/mac.c >> +++ b/drivers/net/wireless/realtek/rtw89/mac.c >> @@ -88,7 +88,7 @@ int rtw89_mac_write_lte(struct rtw89_dev *rtwdev, const u32 offset, u32 val) >> >> ret = read_poll_timeout(rtw89_read8, lte_ctrl, (lte_ctrl & BIT(5)) != 0, >> 50, 50000, false, rtwdev, R_AX_LTE_CTRL + 3); > > For this case, timeout time is large enough for USB. But I'm surprising that > you don't need to adjust timeout time of read_poll_timeout() for USB devices, > since USB is much slower than PCIE. > I don't know, it seems fine. > If sometime you need, I suggest this pattern (number is artificial): > > u64 rtw89_hci_timeout(rtwdev, to) > { > if (USB 2) > return max(to, 200); // I assume USB 2 is slower and 200 is enough for two times IO. > else if (USB 3) > return max(to, 100); // I assume USB 3 is faster than USB 2 > > return to; > } > > u64 to; > > to = rtw89_hci_timeout(rtwdev, 30); > > - read_poll_timeout(..., 1, 30, ...); > + read_poll_timeout(..., 1, to, ...); > >
diff --git a/drivers/net/wireless/realtek/rtw89/core.h b/drivers/net/wireless/realtek/rtw89/core.h index f17f046e773f..015eb38a5a14 100644 --- a/drivers/net/wireless/realtek/rtw89/core.h +++ b/drivers/net/wireless/realtek/rtw89/core.h @@ -4895,6 +4895,7 @@ enum rtw89_flags { RTW89_FLAG_FORBIDDEN_TRACK_WROK, RTW89_FLAG_CHANGING_INTERFACE, RTW89_FLAG_HW_RFKILL_STATE, + RTW89_FLAG_UNPLUGGED, NUM_OF_RTW89_FLAGS, }; diff --git a/drivers/net/wireless/realtek/rtw89/mac.c b/drivers/net/wireless/realtek/rtw89/mac.c index 1a03355b340f..99f01fff90fe 100644 --- a/drivers/net/wireless/realtek/rtw89/mac.c +++ b/drivers/net/wireless/realtek/rtw89/mac.c @@ -88,7 +88,7 @@ int rtw89_mac_write_lte(struct rtw89_dev *rtwdev, const u32 offset, u32 val) ret = read_poll_timeout(rtw89_read8, lte_ctrl, (lte_ctrl & BIT(5)) != 0, 50, 50000, false, rtwdev, R_AX_LTE_CTRL + 3); - if (ret) + if (ret && !test_bit(RTW89_FLAG_UNPLUGGED, rtwdev->flags)) rtw89_err(rtwdev, "[ERR]lte not ready(W)\n"); rtw89_write32(rtwdev, R_AX_LTE_WDATA, val); @@ -104,7 +104,7 @@ int rtw89_mac_read_lte(struct rtw89_dev *rtwdev, const u32 offset, u32 *val) ret = read_poll_timeout(rtw89_read8, lte_ctrl, (lte_ctrl & BIT(5)) != 0, 50, 50000, false, rtwdev, R_AX_LTE_CTRL + 3); - if (ret) + if (ret && !test_bit(RTW89_FLAG_UNPLUGGED, rtwdev->flags)) rtw89_err(rtwdev, "[ERR]lte not ready(W)\n"); rtw89_write32(rtwdev, R_AX_LTE_CTRL, 0x800F0000 | offset); @@ -5842,13 +5842,15 @@ int rtw89_mac_coex_init(struct rtw89_dev *rtwdev, const struct rtw89_mac_ax_coex ret = rtw89_mac_read_lte(rtwdev, R_AX_LTE_SW_CFG_2, &val32); if (ret) { - rtw89_err(rtwdev, "Read R_AX_LTE_SW_CFG_2 fail!\n"); + if (!test_bit(RTW89_FLAG_UNPLUGGED, rtwdev->flags)) + rtw89_err(rtwdev, "Read R_AX_LTE_SW_CFG_2 fail!\n"); return ret; } val32 = val32 & B_AX_WL_RX_CTRL; ret = rtw89_mac_write_lte(rtwdev, R_AX_LTE_SW_CFG_2, val32); if (ret) { - rtw89_err(rtwdev, "Write R_AX_LTE_SW_CFG_2 fail!\n"); + if (!test_bit(RTW89_FLAG_UNPLUGGED, rtwdev->flags)) + rtw89_err(rtwdev, "Write R_AX_LTE_SW_CFG_2 fail!\n"); return ret; } @@ -5972,7 +5974,8 @@ int rtw89_mac_cfg_gnt(struct rtw89_dev *rtwdev, ret = rtw89_mac_write_lte(rtwdev, R_AX_LTE_SW_CFG_1, val); if (ret) { - rtw89_err(rtwdev, "Write LTE fail!\n"); + if (!test_bit(RTW89_FLAG_UNPLUGGED, rtwdev->flags)) + rtw89_err(rtwdev, "Write LTE fail!\n"); return ret; } diff --git a/drivers/net/wireless/realtek/rtw89/phy.c b/drivers/net/wireless/realtek/rtw89/phy.c index 270f40e44c0b..dfff46b92730 100644 --- a/drivers/net/wireless/realtek/rtw89/phy.c +++ b/drivers/net/wireless/realtek/rtw89/phy.c @@ -895,7 +895,8 @@ static u32 rtw89_phy_read_rf_a(struct rtw89_dev *rtwdev, 30, false, rtwdev, R_SWSI_V1, B_SWSI_R_DATA_DONE_V1); if (ret) { - rtw89_err(rtwdev, "read swsi busy\n"); + if (!test_bit(RTW89_FLAG_UNPLUGGED, rtwdev->flags)) + rtw89_err(rtwdev, "read swsi busy\n"); return INV_RF_DATA; }
A few unnecessary error messages are printed when the device is unplugged. "read swsi busy" in particular can appear ~1000 times when RTL8851BU is unplugged. Add a new flag RTW89_FLAG_UNPLUGGED and print some error messages only when this flag is not set. The new USB driver will set the flag when the device is unplugged. Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com> --- drivers/net/wireless/realtek/rtw89/core.h | 1 + drivers/net/wireless/realtek/rtw89/mac.c | 13 ++++++++----- drivers/net/wireless/realtek/rtw89/phy.c | 3 ++- 3 files changed, 11 insertions(+), 6 deletions(-)