RE: [PATCH rtw-next v7 4/6] wifi: rtw88: 8723b: add the RTL8723B chip driver

[Ping-Ke Shih]

Luka Gejak <luka.gejak@xxxxxxxxx> wrote:
> Add the Realtek RTL8723B 802.11n chip driver: the chip operations, the
> power sequences, the efuse layout, the RF and IQ calibration, and the
> chip specific coexistence handling.
> 
> The RTL8723B chip support is based on the initial work by
> Michael Straube <straube.linux@xxxxxxxxx>.
> Link: https://github.com/mistraube/rtw88/tree/rtl8723bs
> 
> Co-developed-by: Michael Straube <straube.linux@xxxxxxxxx>
> Signed-off-by: Michael Straube <straube.linux@xxxxxxxxx>
> Signed-off-by: Luka Gejak <luka.gejak@xxxxxxxxx>
> ---
>  drivers/net/wireless/realtek/rtw88/rtw8723b.c | 2550 +++++++++++++++++
>  drivers/net/wireless/realtek/rtw88/rtw8723b.h |   14 +
>  2 files changed, 2564 insertions(+)
>  create mode 100644 drivers/net/wireless/realtek/rtw88/rtw8723b.c
>  create mode 100644 drivers/net/wireless/realtek/rtw88/rtw8723b.h
> 
> diff --git a/drivers/net/wireless/realtek/rtw88/rtw8723b.c
> b/drivers/net/wireless/realtek/rtw88/rtw8723b.c
> new file mode 100644
> index 000000000000..8791cd2e0e27
> --- /dev/null
> +++ b/drivers/net/wireless/realtek/rtw88/rtw8723b.c
> @@ -0,0 +1,2550 @@
> +// SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause
> +/* Copyright(c) 2026  Realtek Corporation
> + * Copyright(c) Michael Straube <straube.linux@xxxxxxxxx>
> + * Copyright(c) 2024-2026 Luka Gejak <luka.gejak@xxxxxxxxx>
> + */
> +
> +#include "main.h"
> +#include "coex.h"
> +#include "fw.h"
> +#include "mac.h"
> +#include "phy.h"
> +/*
> + * Shares the receive PHY status layout, the SDIO aggregation burst fields
> + * and a few baseband registers with the RTL8703B; reuse that header.
> + */
> +#include "rtw8703b.h"

Which layout you are using?
Should you move the layout to rtw8723x.h ?

> +/*
> + * Row 20 (-6.0 dB) intentionally does not match the v5.2.17 vendor driver,

I really don't want to mention vendor driver here. If you really need it,
mention it in commit message or cover-letter.

> + * which has 0x1c, 0x1a, 0x18, 0x12, 0x0e, 0x08 there. Every other row agrees.
> + * The values below are what rtl8723be, the mainline driver for this same
> + * chip, uses at the same index, and they are also what the vendor's own
> + * cck_swing_table_ch1_ch13_92e and the staging rtl8723bs driver use. They
> + * also track the 0.5 dB step of the surrounding rows: against row 32 as 0 dB,
> + * 0x1b is within 0.06 of the ideal -6.0 dB value while 0x1c is 0.94 away,
> + * the largest error anywhere in the table. Treat the vendor row as the
> + * anomaly and do not "fix" this towards it.

And you have comments each row. Is it still need this block comment to explain?

> + */
> +static const u8 rtw8723b_cck_swing_table_ch1_ch13[][8] = {
> +       {0x09, 0x08, 0x07, 0x06, 0x04, 0x03, 0x01, 0x01},       /* 0, -16.0dB */
> +       {0x09, 0x09, 0x08, 0x06, 0x05, 0x03, 0x01, 0x01},       /* 1, -15.5dB */
> +       {0x0a, 0x09, 0x08, 0x07, 0x05, 0x03, 0x02, 0x01},       /* 2, -15.0dB */
> +       {0x0a, 0x0a, 0x09, 0x07, 0x05, 0x03, 0x02, 0x01},       /* 3, -14.5dB */
> +       {0x0b, 0x0a, 0x09, 0x08, 0x06, 0x04, 0x02, 0x01},       /* 4, -14.0dB */
> +       {0x0b, 0x0b, 0x0a, 0x08, 0x06, 0x04, 0x02, 0x01},       /* 5, -13.5dB */
> +       {0x0c, 0x0c, 0x0a, 0x09, 0x06, 0x04, 0x02, 0x01},       /* 6, -13.0dB */
> +       {0x0d, 0x0c, 0x0b, 0x09, 0x07, 0x04, 0x02, 0x01},       /* 7, -12.5dB */
> +       {0x0d, 0x0d, 0x0c, 0x0a, 0x07, 0x05, 0x02, 0x01},       /* 8, -12.0dB */
> +       {0x0e, 0x0e, 0x0c, 0x0a, 0x08, 0x05, 0x02, 0x01},       /* 9, -11.5dB */
> +       {0x0f, 0x0f, 0x0d, 0x0b, 0x08, 0x05, 0x03, 0x01},       /* 10, -11.0dB */
> +       {0x10, 0x10, 0x0e, 0x0b, 0x08, 0x05, 0x03, 0x01},       /* 11, -10.5dB */
> +       {0x11, 0x11, 0x0f, 0x0c, 0x09, 0x06, 0x03, 0x01},       /* 12, -10.0dB */
> +       {0x12, 0x12, 0x0f, 0x0c, 0x09, 0x06, 0x03, 0x01},       /* 13, -9.5dB */
> +       {0x13, 0x13, 0x10, 0x0d, 0x0a, 0x06, 0x03, 0x01},       /* 14, -9.0dB */
> +       {0x14, 0x14, 0x11, 0x0e, 0x0b, 0x07, 0x03, 0x02},       /* 15, -8.5dB */
> +       {0x16, 0x15, 0x12, 0x0f, 0x0b, 0x07, 0x04, 0x01},       /* 16, -8.0dB */
> +       {0x17, 0x16, 0x13, 0x10, 0x0c, 0x08, 0x04, 0x02},       /* 17, -7.5dB */
> +       {0x18, 0x17, 0x15, 0x11, 0x0c, 0x08, 0x04, 0x02},       /* 18, -7.0dB */
> +       {0x1a, 0x19, 0x16, 0x12, 0x0d, 0x09, 0x04, 0x02},       /* 19, -6.5dB */
> +       {0x1b, 0x1a, 0x17, 0x13, 0x0e, 0x09, 0x04, 0x02},       /* 20, -6.0dB */
> +       {0x1d, 0x1c, 0x18, 0x14, 0x0f, 0x0a, 0x05, 0x02},       /* 21, -5.5dB */
> +       {0x1f, 0x1e, 0x1a, 0x15, 0x10, 0x0a, 0x05, 0x02},       /* 22, -5.0dB */
> +       {0x20, 0x20, 0x1b, 0x16, 0x11, 0x08, 0x05, 0x02},       /* 23, -4.5dB */
> +       {0x22, 0x21, 0x1d, 0x18, 0x11, 0x0b, 0x06, 0x02},       /* 24, -4.0dB */
> +       {0x24, 0x23, 0x1f, 0x19, 0x13, 0x0c, 0x06, 0x03},       /* 25, -3.5dB */
> +       {0x26, 0x25, 0x21, 0x1b, 0x14, 0x0d, 0x06, 0x03},       /* 26, -3.0dB */
> +       {0x28, 0x28, 0x22, 0x1c, 0x15, 0x0d, 0x07, 0x03},       /* 27, -2.5dB */
> +       {0x2b, 0x2a, 0x25, 0x1e, 0x16, 0x0e, 0x07, 0x03},       /* 28, -2.0dB */
> +       {0x2d, 0x2d, 0x27, 0x1f, 0x18, 0x0f, 0x08, 0x03},       /* 29, -1.5dB */
> +       {0x30, 0x2f, 0x29, 0x21, 0x19, 0x10, 0x08, 0x03},       /* 30, -1.0dB */
> +       {0x33, 0x32, 0x2b, 0x23, 0x1a, 0x11, 0x08, 0x04},       /* 31, -0.5dB */
> +       {0x36, 0x35, 0x2e, 0x25, 0x1c, 0x12, 0x09, 0x04},       /* 32, +0dB */
> +};
> +

[...]


> +
> +static void rtw8723b_query_phy_status_cck(struct rtw_dev *rtwdev, u8 *phy_raw,
> +                                         struct rtw_rx_pkt_stat *pkt_stat)
> +{
> +       struct phy_status_8703b *phy_status = (struct phy_status_8703b *)phy_raw;

nit: Avoid casting by argument 'void *phy_raw'. 

> +       u8 lna_idx = (phy_status->cck_agc_rpt_ofdm_cfosho_a & 0xE0) >> 5;
> +       u8 vga_idx = (phy_status->cck_agc_rpt_ofdm_cfosho_a & 0x1F);

u8_get_bits()

> +       s8 rx_power = rtw8723b_cck_rx_power(lna_idx, vga_idx);
> +       s8 min_rx_power = -120;
> +
> +       pkt_stat->bw = RTW_CHANNEL_WIDTH_20;
> +
> +       pkt_stat->rx_power[RF_PATH_A] = rx_power;
> +       pkt_stat->rssi = rtw_phy_rf_power_2_rssi(pkt_stat->rx_power, 1);
> +       pkt_stat->signal_power = max(pkt_stat->rx_power[RF_PATH_A],
> +                                    min_rx_power);
> +       rtwdev->dm_info.rssi[RF_PATH_A] = pkt_stat->rssi;
> +}
> +
> +static void rtw8723b_query_phy_status_ofdm(struct rtw_dev *rtwdev, u8 *phy_raw,
> +                                          struct rtw_rx_pkt_stat *pkt_stat)
> +{
> +       struct phy_status_8703b *phy_status = (struct phy_status_8703b *)phy_raw;

ditto. (void *phy_raw)

> +
> +static const struct rtw_chip_ops rtw8723b_ops = {
> +       .power_on               = rtw_power_on,
> +       .power_off              = rtw_power_off,
> +
> +       .mac_init               = rtw8723x_mac_init,
> +       .mac_postinit           = rtw8723x_mac_postinit,
> +
> +       .dump_fw_crash          = NULL,
> +       /*
> +        * 8723d sets REG_HCI_OPT_CTRL BIT_USB_SUS_DIS in its shutdown
> +        * function; that is USB-only.
> +        */
> +       .shutdown               = NULL,
> +       .read_efuse             = rtw8723b_read_efuse,
> +       .phy_set_param          = rtw8723b_phy_set_param,
> +
> +       .set_channel            = rtw8723b_set_channel,
> +
> +       .query_phy_status       = rtw8723b_query_phy_status,
> +       .read_rf                = rtw_phy_read_rf_sipi,
> +       .write_rf               = rtw_phy_write_rf_reg_sipi,
> +       .set_tx_power_index     = rtw8723x_set_tx_power_index,
> +       .rsvd_page_dump         = NULL,
> +       .set_antenna            = NULL,
> +       .cfg_ldo25              = rtw8723b_cfg_ldo25,
> +       .efuse_grant            = rtw8723b_efuse_grant,
> +       .set_ampdu_factor       = NULL,
> +       .false_alarm_statistics = rtw8723x_false_alarm_statistics,
> +       .phy_calibration        = rtw8723b_phy_calibration,
> +       .dpk_track              = NULL,
> +       .cck_pd_set             = rtw_phy_cck_pd_set,
> +       .pwr_track              = rtw8723b_pwr_track,
> +       .config_bfee            = NULL,
> +       .set_gid_table          = NULL,
> +       .cfg_csi_rate           = NULL,
> +       .adaptivity_init        = NULL,
> +       .adaptivity             = NULL,
> +       .cfo_init               = NULL,
> +       .cfo_track              = NULL,
> +       .config_tx_path         = NULL,
> +       .config_txrx_mode       = NULL,
> +       .led_set                = NULL,
> +       .fill_txdesc_checksum   = rtw8723b_fill_txdesc_checksum,
> +
> +       .coex_set_init          = rtw8723b_coex_cfg_init,
> +       .coex_set_ant_switch    = rtw8723b_coex_cfg_ant_switch,
> +       .coex_set_gnt_fix       = rtw8723b_coex_set_gnt_fix,
> +       .coex_set_gnt_debug     = rtw8723b_coex_set_gnt_debug,
> +       .coex_set_rfe_type      = rtw8723b_coex_set_rfe_type,
> +       .coex_set_wl_tx_power   = rtw8723b_coex_set_wl_tx_power,
> +       .coex_set_wl_rx_gain    = rtw8723b_coex_set_wl_rx_gain,
> +};
> +
> +const struct rtw_chip_info rtw8723b_hw_spec = {
> +       .ops = &rtw8723b_ops,
> +       .id = RTW_CHIP_TYPE_8723B,
> +       .fw_name = "rtw88/rtw8723b_fw.bin",
> +       .wlan_cpu = RTW_WCPU_8051,
> +       .tx_pkt_desc_sz = 40,
> +       .tx_buf_desc_sz = 16,
> +       .rx_pkt_desc_sz = 24,
> +       .rx_buf_desc_sz = 8,
> +       .phy_efuse_size = 512,
> +       .log_efuse_size = 512,
> +       .ptct_efuse_size = 15,
> +       .txff_size = 32768,
> +       .rxff_size = 16384,
> +       .rsvd_drv_pg_num = 8,
> +       .txgi_factor = 1,
> +       .is_pwr_by_rate_dec = true,
> +       .max_power_index = 0x3f,
> +       .csi_buf_pg_num = 0,
> +       .band = RTW_BAND_2G,
> +       .page_size = TX_PAGE_SIZE,
> +       .dig_min = 0x20,
> +       .usb_tx_agg_desc_num = 6,
> +       /*
> +        * The firmware reports id 0xfd instead of C2H_HW_FEATURE_REPORT, so
> +        * the hardware feature report is not supported on this chip.
> +        */
> +       .hw_feature_report = false,
> +       .c2h_ra_report_size = 4,
> +       .old_datarate_fb_limit = true,
> +       .path_div_supported = false,
> +       .ht_supported = true,
> +       .vht_supported = false,
> +       .lps_deep_mode_supported = 0,
> +       .sys_func_en = 0xfd,
> +       .pwr_on_seq = card_enable_flow_8723b,
> +       .pwr_off_seq = card_disable_flow_8723b,
> +       .page_table = page_table_8723b,
> +       .rqpn_table = rqpn_table_8723b,
> +       /* same shared table as the sibling rtw8703b and rtw8723d */
> +       .prioq_addrs = &rtw8723x_common.prioq_addrs,
> +       /* used only in pci.c, not needed for SDIO devices */
> +       .intf_table = NULL,
> +       .dig = rtw8723x_common.dig,
> +       /* The vendor driver never writes the CCK IGI on this chip. */
> +       .dig_cck = NULL,
> +       .rf_sipi_addr = {0x840, 0x844},
> +       .rf_sipi_read_addr = rtw8723x_common.rf_sipi_addr,
> +       .fix_rf_phy_num = 2,
> +       /* This chip has no LTE coex registers. */
> +       .ltecoex_addr = NULL,
> +       .mac_tbl = &rtw8723b_mac_tbl,
> +       .agc_tbl = &rtw8723b_agc_tbl,
> +       .bb_tbl = &rtw8723b_bb_tbl,
> +       .rf_tbl = {&rtw8723b_rf_a_tbl},
> +       .rfe_defs = rtw8723b_rfe_defs,
> +       .rfe_defs_size = ARRAY_SIZE(rtw8723b_rfe_defs),
> +       .iqk_threshold = 8,
> +       .rx_ldpc = false,
> +       .tx_stbc = false,
> +       .ampdu_density = IEEE80211_HT_MPDU_DENSITY_16,
> +       .max_scan_ie_len = IEEE80211_MAX_DATA_LEN,
> +       .coex_para_ver = 20180201,      /* glcoex_ver_date_8723b_1ant */
> +       .bt_desired_ver = 0x6d,
> +       .scbd_support = false,
> +       .new_scbd10_def = true,
> +       .ble_hid_profile_support = false,
> +       .wl_mimo_ps_support = false,
> +       .pstdma_type = COEX_PSTDMA_FORCE_LPSOFF,
> +       .bt_rssi_type = COEX_BTRSSI_RATIO,
> +       .ant_isolation = 15,
> +       .rssi_tolerance = 2,
> +       .wl_rssi_step = wl_rssi_step_8723b,
> +       .bt_rssi_step = bt_rssi_step_8723b,
> +       .table_sant_num = ARRAY_SIZE(table_sant_8723b),
> +       .table_sant = table_sant_8723b,
> +       .table_nsant_num = ARRAY_SIZE(table_nsant_8723b),
> +       .table_nsant = table_nsant_8723b,
> +       .tdma_sant_num = ARRAY_SIZE(tdma_sant_8723b),
> +       .tdma_sant = tdma_sant_8723b,
> +       .tdma_nsant_num = ARRAY_SIZE(tdma_nsant_8723b),
> +       .tdma_nsant = tdma_nsant_8723b,
> +       .wl_rf_para_num = ARRAY_SIZE(rf_para_tx_8723b),
> +       .wl_rf_para_tx = rf_para_tx_8723b,
> +       .wl_rf_para_rx = rf_para_rx_8723b,
> +       .bt_afh_span_bw20 = 0x20,
> +       .bt_afh_span_bw40 = 0x30,
> +       .afh_5g_num =  ARRAY_SIZE(afh_5g_8723b),
> +       .afh_5g = afh_5g_8723b,
> +       /* BTG_SEL is driven by the cardemu_to_act power sequence instead. */
> +       .btg_reg = NULL,
> +       .coex_info_hw_regs_num = 0,
> +       .coex_info_hw_regs = NULL,
> +};
> +EXPORT_SYMBOL(rtw8723b_hw_spec);

I guess you copy these two tables from somewhere and modify the values.
However, when I compare these with RTL8822C's ones. The order is very
different... Can you align the order?

Realtek WiFi chips are different from one to another, and we add many
parameters to support the variants. To prevent the order being messed
up, I ask people to add dummy (unused) fields (e.g. .xxx = NULL, .yyy = 0)
to keep the order and consistent. But now, rtw88 becomes very different
again. 

Let me know your source, I'd think how we can align them sometime.