Hi Tony, > On Apr 6, 2020, at 19:01, Tony Chuang <yhchuang@xxxxxxxxxxx> wrote: > >> Subject: [PATCH] rtw88: Add delay on polling h2c command status bit >> >> On some systems we can constanly see rtw88 complains: >> [39584.721375] rtw_pci 0000:03:00.0: failed to send h2c command >> >> Increase interval of each check to wait the status bit really changes. >> >> While at it, add some helpers so we can use standarized >> readx_poll_timeout() macro. >> >> Signed-off-by: Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx> >> --- >> drivers/net/wireless/realtek/rtw88/fw.c | 12 ++++++------ >> drivers/net/wireless/realtek/rtw88/hci.h | 4 ++++ >> 2 files changed, 10 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/net/wireless/realtek/rtw88/fw.c >> b/drivers/net/wireless/realtek/rtw88/fw.c >> index 05c430b3489c..bc9982e77524 100644 >> --- a/drivers/net/wireless/realtek/rtw88/fw.c >> +++ b/drivers/net/wireless/realtek/rtw88/fw.c >> @@ -2,6 +2,8 @@ >> /* Copyright(c) 2018-2019 Realtek Corporation >> */ >> >> +#include <linux/iopoll.h> >> + >> #include "main.h" >> #include "coex.h" >> #include "fw.h" >> @@ -193,8 +195,8 @@ static void rtw_fw_send_h2c_command(struct >> rtw_dev *rtwdev, >> u8 box; >> u8 box_state; >> u32 box_reg, box_ex_reg; >> - u32 h2c_wait; >> int idx; >> + int ret; >> >> rtw_dbg(rtwdev, RTW_DBG_FW, >> "send H2C content %02x%02x%02x%02x %02x%02x%02x%02x\n", >> @@ -226,12 +228,10 @@ static void rtw_fw_send_h2c_command(struct >> rtw_dev *rtwdev, >> goto out; >> } >> >> - h2c_wait = 20; >> - do { >> - box_state = rtw_read8(rtwdev, REG_HMETFR); >> - } while ((box_state >> box) & 0x1 && --h2c_wait > 0); >> + ret = readx_poll_timeout(rr8, REG_HMETFR, box_state, >> + !((box_state >> box) & 0x1), 100, 3000); >> >> - if (!h2c_wait) { >> + if (ret) { >> rtw_err(rtwdev, "failed to send h2c command\n"); >> goto out; >> } >> diff --git a/drivers/net/wireless/realtek/rtw88/hci.h >> b/drivers/net/wireless/realtek/rtw88/hci.h >> index 2cba327e6218..24062c7079c6 100644 >> --- a/drivers/net/wireless/realtek/rtw88/hci.h >> +++ b/drivers/net/wireless/realtek/rtw88/hci.h >> @@ -253,6 +253,10 @@ rtw_write8_mask(struct rtw_dev *rtwdev, u32 addr, >> u32 mask, u8 data) >> rtw_write8(rtwdev, addr, set); >> } >> >> +#define rr8(addr) rtw_read8(rtwdev, addr) >> +#define rr16(addr) rtw_read16(rtwdev, addr) >> +#define rr32(addr) rtw_read32(rtwdev, addr) >> + >> static inline enum rtw_hci_type rtw_hci_type(struct rtw_dev *rtwdev) >> { >> return rtwdev->hci.type; >> -- > > I think the timeout is because the H2C is triggered when the lower 4 bytes are written. > So, we probably should write h2c[4] ~ h2c[7] before h2c[0] ~ h2c[3]. I can still see "failed to send h2c command" with the following patch: diff --git a/drivers/net/wireless/realtek/rtw88/fw.c b/drivers/net/wireless/realtek/rtw88/fw.c index eb7e623c811a..a296c860045f 100644 --- a/drivers/net/wireless/realtek/rtw88/fw.c +++ b/drivers/net/wireless/realtek/rtw88/fw.c @@ -240,10 +240,10 @@ static void rtw_fw_send_h2c_command(struct rtw_dev *rtwdev, goto out; } - for (idx = 0; idx < 4; idx++) - rtw_write8(rtwdev, box_reg + idx, h2c[idx]); for (idx = 0; idx < 4; idx++) rtw_write8(rtwdev, box_ex_reg + idx, h2c[idx + 4]); + for (idx = 0; idx < 4; idx++) + rtw_write8(rtwdev, box_reg + idx, h2c[idx]); if (++rtwdev->h2c.last_box_num >= 4) rtwdev->h2c.last_box_num = 0; > > But this delay still works, I think you can keep it, and reorder the h2c write sequence. > > Yen-Hsuan