On 21/02/2023 04:45, Ping-Ke Shih wrote: > > >> -----Original Message----- >> From: Bitterblue Smith <rtl8821cerfe2@xxxxxxxxx> >> Sent: Tuesday, February 21, 2023 12:11 AM >> To: linux-wireless@xxxxxxxxxxxxxxx >> Cc: Jes Sorensen <Jes.Sorensen@xxxxxxxxx>; Ping-Ke Shih <pkshih@xxxxxxxxxxx> >> Subject: [PATCH] wifi: rtl8xxxu: Support new chip RTL8710BU aka RTL8188GU >> >> This chip is found in cheap "free driver" USB adapters from Aliexpress. >> Initially they pretend to be a CD-ROM containing the driver for Windows. >> "Ejecting" switches the device to wifi mode. >> >> Features: 2.4 GHz, b/g/n mode, 1T1R, 150 Mbps. >> >> This chip is more unique than other Realtek chips: >> >> * The registers at addresses 0x0-0xff, which all the other chips use, >> can't be used here. New registers at 0x8000-0x80ff must be used >> instead. And it's not a simple matter of adding 0x8000: 0x2 >> (REG_SYS_FUNC) became 0x8004, 0x80 (REG_MCU_FW_DL) became 0x8090, >> etc. >> >> * Also there are a few new registers which must be accessed indirectly >> because their addresses don't fit in 16 bits. No other chips seem to >> have these. >> >> * The vendor driver compiles to 8188gu.ko, but the code calls the chip >> RTL8710B(U) pretty much everywhere, including messages visible to the >> user. >> >> Another difference compared to the other chips supported by rtl8xxxu is >> that it has a new PHY status struct, or three of them actually, from >> which we extract the RSSI, among other things. This is not unique, >> though, just new. The chips supported by rtw88 also use it. >> >> This was tested with a device from vendor SMIC with romver 1. It's >> possible devices from vendor UMC and/or with romver 2 will fail to >> upload the firmware. The vendor driver has that problem. > > Can I know the version of vendor driver you use? > I have v5.2.20.2_28373.20180619 from here: https://github.com/McMCCRU/rtl8188gu Most likely it's a copy of Larry Finger's repo--with some fixes, because the driver Larry Finger received from Realtek had some problems (unrelated to the firmware upload issue). https://github.com/lwfinger/rtl8188gu >> >> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@xxxxxxxxx> >> --- >> drivers/net/wireless/realtek/rtl8xxxu/Kconfig | 2 +- >> .../net/wireless/realtek/rtl8xxxu/Makefile | 2 +- >> .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 304 ++- >> .../realtek/rtl8xxxu/rtl8xxxu_8188e.c | 2 + >> .../realtek/rtl8xxxu/rtl8xxxu_8188f.c | 4 +- >> .../realtek/rtl8xxxu/rtl8xxxu_8192c.c | 2 + >> .../realtek/rtl8xxxu/rtl8xxxu_8192e.c | 2 + >> .../realtek/rtl8xxxu/rtl8xxxu_8710b.c | 1942 +++++++++++++++++ >> .../realtek/rtl8xxxu/rtl8xxxu_8723a.c | 2 + >> .../realtek/rtl8xxxu/rtl8xxxu_8723b.c | 2 + >> .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 181 +- >> .../wireless/realtek/rtl8xxxu/rtl8xxxu_regs.h | 43 + >> 12 files changed, 2435 insertions(+), 53 deletions(-) >> create mode 100644 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8710b.c >> > > [...] > >> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h >> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h >> index c8cee4a24755..2e5fa9479134 100644 >> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h >> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h >> @@ -103,7 +103,8 @@ enum rtl8xxxu_rtl_chip { >> RTL8822B = 0x8822b, >> RTL8703B = 0x8703b, >> RTL8195A = 0x8195a, >> - RTL8188F = 0x8188f >> + RTL8188F = 0x8188f, >> + RTL8710B = 0x8710b > > Add a trailing comma, and then you don't need to stir the last line when adding > an new chip in the future. > > [...] > >> struct rtl8xxxu_reg8val { >> u16 reg; >> u8 val; >> @@ -1486,6 +1769,7 @@ struct rtl8xxxu_priv { >> struct rtl8723au_idx ht20_tx_power_diff[RTL8723B_TX_COUNT]; >> struct rtl8723au_idx ht40_tx_power_diff[RTL8723B_TX_COUNT]; >> struct rtl8xxxu_power_base *power_base; >> + u8 package_type; >> u32 chip_cut:4; >> u32 rom_rev:4; >> u32 is_multi_func:1; >> @@ -1505,6 +1789,7 @@ struct rtl8xxxu_priv { >> u32 ep_tx_low_queue:1; >> u32 rx_buf_aggregation:1; >> u32 cck_agc_report_type:1; >> + u32 cck_new_agc:1; >> u8 default_crystal_cap; >> unsigned int pipe_interrupt; >> unsigned int pipe_in; >> @@ -1523,6 +1808,8 @@ struct rtl8xxxu_priv { >> >> struct mutex h2c_mutex; >> >> + struct mutex syson_indirect_access_mutex; >> + > > I remember checkpatch will warn if you don't add a comment for mutex/lock. > Though the naming is clear, just follow the rule. > > [...] > >> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8710b.c >> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8710b.c >> new file mode 100644 >> index 000000000000..d80125157514 >> --- /dev/null >> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8710b.c > > [...] > >> +static u32 rtl8710b_indirect_read32(struct rtl8xxxu_priv *priv, u32 addr) >> +{ >> + struct device *dev = &priv->udev->dev; >> + u32 val32, value = 0xffffffff; >> + u8 polling_count = 0xff; >> + >> + if (addr & 3) { >> + dev_warn(dev, "%s: Aborting because 0x%x is not a multiple of 4.\n", >> + __func__, addr); >> + return value; >> + } >> + >> + mutex_lock(&priv->syson_indirect_access_mutex); >> + >> + rtl8xxxu_write32(priv, REG_USB_HOST_INDIRECT_ADDR_8710B, addr); >> + rtl8xxxu_write32(priv, REG_EFUSE_INDIRECT_CTRL_8710B, NORMAL_REG_READ_OFFSET); >> + >> + do >> + val32 = rtl8xxxu_read32(priv, REG_EFUSE_INDIRECT_CTRL_8710B); >> + while ((val32 & BIT(31)) && (--polling_count > 0)); > > Without a pair of brace, that looks a little odd to me, but the result should > be correct. > I think the rule is that we should not use braces when there is a single line inside. I assumed that applies to do...while loops too. > [...] > >> +static int rtl8710bu_identify_chip(struct rtl8xxxu_priv *priv) >> +{ >> + struct device *dev = &priv->udev->dev; >> + u32 cfg0, cfg2, vendor; >> + u8 package_type = 0x7; /* a nonsense value */ >> + int ret = 0; > > No need '=0'. Even, don't need 'ret'. > Just return rtl8xxxu_config_endpoints_no_sie(); > > [...] > >> + >> + ret = rtl8xxxu_config_endpoints_no_sie(priv); >> + >> + return ret; >> +} >> + >> +static void rtl8710b_revise_cck_tx_psf(struct rtl8xxxu_priv *priv, u8 channel) >> +{ >> + if (channel == 13) { >> + /* Normal values */ >> + rtl8xxxu_write32(priv, REG_CCK0_TX_FILTER2, 0x64B80C1C); >> + rtl8xxxu_write32(priv, REG_CCK0_DEBUG_PORT, 0x00008810); >> + rtl8xxxu_write32(priv, 0xaac, 0x01235667); > > 0xaac is another TX filter, so > #define REG_CCK0_TX_FILTER3 0x0aac > >> + /* Special value for channel 13 */ >> + rtl8xxxu_write32(priv, REG_CCK0_TX_FILTER1, 0xd1d80001); >> + } else if (channel == 14) { >> + /* Special values for channel 14 */ >> + rtl8xxxu_write32(priv, REG_CCK0_TX_FILTER2, 0x0000B81C); >> + rtl8xxxu_write32(priv, REG_CCK0_DEBUG_PORT, 0x00000000); >> + rtl8xxxu_write32(priv, 0xaac, 0x00003667); >> + /* Normal value */ >> + rtl8xxxu_write32(priv, REG_CCK0_TX_FILTER1, 0xE82C0001); >> + } else { >> + /* Restore normal values from the phy init table */ >> + rtl8xxxu_write32(priv, REG_CCK0_TX_FILTER2, 0x64B80C1C); >> + rtl8xxxu_write32(priv, REG_CCK0_DEBUG_PORT, 0x00008810); >> + rtl8xxxu_write32(priv, 0xaac, 0x01235667); >> + rtl8xxxu_write32(priv, REG_CCK0_TX_FILTER1, 0xE82C0001); >> + } >> +} >> + > > [...] > >> +static int rtl8710b_read_efuse(struct rtl8xxxu_priv *priv) >> +{ >> + struct device *dev = &priv->udev->dev; >> + int i, ret = 0; > > move these two downward to 'val32'. Declarations should be in reverse > Xmas tree order. > >> + u8 val8, word_mask, header, extheader; >> + u16 efuse_addr, offset; >> + u32 val32; >> + >> + val32 = rtl8710b_read_syson_reg(priv, REG_SYS_EEPROM_CTRL0_8710B); >> + priv->boot_eeprom = u32_get_bits(val32, EEPROM_BOOT); >> + priv->has_eeprom = u32_get_bits(val32, EEPROM_ENABLE); >> + > > [...] > >> +static int rtl8710bu_active_to_lps(struct rtl8xxxu_priv *priv) >> +{ >> + struct device *dev = &priv->udev->dev; >> + u8 val8; >> + u16 val16; >> + u32 val32; >> + int retry, retval; >> + >> + /* Tx Pause */ >> + rtl8xxxu_write8(priv, REG_TXPAUSE, 0xff); >> + >> + retry = 100; >> + retval = -EBUSY; >> + /* >> + * Poll 32 bit wide REG_SCH_TX_CMD for 0x00000000 to ensure no TX is pending. >> + */ >> + do { >> + val32 = rtl8xxxu_read32(priv, REG_SCH_TX_CMD); >> + if (!val32) { >> + retval = 0; >> + break; >> + } >> + udelay(10); >> + } while (retry--); >> + >> + if (!retry) { >> + dev_warn(dev, "Failed to flush TX queue\n"); >> + retval = -EBUSY; >> + return retval; >> + } >> + >> + /* Disable CCK and OFDM, clock gated */ >> + val8 = rtl8xxxu_read8(priv, 0x02); >> + val8 &= ~BIT(0); >> + rtl8xxxu_write8(priv, 0x02, val8); > > rtl8xxxu has already defined these: > > #define REG_SYS_FUNC 0x0002 > #define SYS_FUNC_BBRSTB BIT(0) > #define SYS_FUNC_BB_GLB_RSTN BIT(1) > >> + >> + udelay(2); >> + >> + /*Whole BB is reset*/ >> + val8 = rtl8xxxu_read8(priv, 0x02); >> + val8 &= ~BIT(1); >> + rtl8xxxu_write8(priv, 0x02, val8); >> + >> + /* Reset MAC TRX */ >> + val16 = rtl8xxxu_read16(priv, REG_CR); >> + val16 &= 0xff00; >> + val16 |= CR_HCI_RXDMA_ENABLE | CR_HCI_TXDMA_ENABLE; >> + val16 &= ~CR_SECURITY_ENABLE; >> + rtl8xxxu_write16(priv, REG_CR, val16); >> + >> + /*Respond TxOK to scheduler*/ >> + val8 = rtl8xxxu_read8(priv, REG_DUAL_TSF_RST); >> + val8 |= DUAL_TSF_TX_OK; >> + rtl8xxxu_write8(priv, REG_DUAL_TSF_RST, val8); >> + >> + return retval; >> +} > > [...] > >> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c >> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c >> index 620a5cc2bfdd..e223ef852899 100644 >> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c >> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c > > [...] > >> @@ -1904,41 +1929,54 @@ static int rtl8xxxu_download_firmware(struct rtl8xxxu_priv *priv) >> { >> int pages, remainder, i, ret; >> u8 val8; >> - u16 val16; >> + u16 val16, reg_mcu_fw_dl; > > nit: define reg_mcu_fw_dl separately > >> u32 val32; >> u8 *fwptr; >> >> - val8 = rtl8xxxu_read8(priv, REG_SYS_FUNC + 1); >> - val8 |= 4; >> - rtl8xxxu_write8(priv, REG_SYS_FUNC + 1, val8); >> + if (priv->rtl_chip == RTL8710B) { >> + reg_mcu_fw_dl = REG_8051FW_CTRL_V1_8710B; >> + } else { >> + reg_mcu_fw_dl = REG_MCU_FW_DL; >> > > [...] > > Only some small suggestions. > Thanks for this nice work. > > Ping-Ke > Thank you for your review. I'll make the changes.
