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

["Luka Gejak" <luka.gejak@linux.dev>]

On Mon Oct 5, 2026 at 8:08 AM CEST, Ping-Ke Shih wrote:
> Luka Gejak <luka.gejak@linux.dev> wrote:
[...]
>> +/*
>> + * 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 ?
>

The layout I reuse is the RTL8703B receive PHY status structure, struct
phy_status_8703b, together with the SDIO aggregation burst fields and four
baseband registers. However, including rtw8703b.h from another chip driver is
already established, rtw8723cs.c includes it to reuse rtw8703b_hw_spec. So I
would rather keep the same include here, and if you want the layout in rtw8723x.h
I can send that as a separate patch later, so this series does not touch
additional 2 drivers.

>> +/*
>> + * 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?
>

I agree, will drop vendor reference and block comment.

[...]
>> +
>> +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'. 
>

Understood.

>> +       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()
>

Will switch to it.

>> +       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)
>

Same as above.

>> +
>> +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. 

I took the ordering from the sibling rtw8703b and rtw8723d drivers. rtw8723b_ops
is already in the order of the struct rtw_chip_ops declaration, and it follows the
same order as rtw8703b_ops. I believe rtw8822c_ops is the one that differs, since it
groups the fields by function instead of following the struct. The values were taken
from the v5.2.17 vendor driver and checked against the staging rtl8723bs. So I would
prefer to leave both tables as they are. If you want the unused fields listed as dummies
to pin the order, I can add them, but the sequence itself would not change.

Best regards,
Luka Gejak