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

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

October 6, 2026 at 02:39, "Ping-Ke Shih" <pkshih@realtek.com mailto:pkshih@realtek.com?to=%22Ping-Ke%20Shih%22%20%3Cpkshih%40realtek.com%3E > wrote:


> 
> Luka Gejak <luka.gejak@linux.dev> wrote:
> 
> > 
> > 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.
> > 
> Let's use another patch to move the struct out of rtw8703b.h, and rename
> to phy_status_8723x for example.

Understood, will do.

> 
> > 
> > However, including rtw8703b.h from another chip driver is
> >  already established, rtw8723cs.c includes it to reuse rtw8703b_hw_spec.
> > 
> As I know, 8723CS and 8723B are mutual alias, no?

As far as I know, they are not. They are different chips.

> 
> > 
> > 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.
> > 
> Yes, another patch before this one. 
> 

Ok.

> > 
> > +/*
> >  + * 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.
> > 
> I'm not sure if LLM writes this? LLM always write verbose comments for
> each line it added. Just ask LLM to write self-explained code.

No, I wrote it because only 1 row differs from vendor driver and I
thought I should mention it.

> 
> [...]
> 
> > 
> > +
> >  +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.
> > 
> Okay. Please make sure the ordering are the same as the one you copied. 
> 
> > 
> > The values were taken
> >  from the v5.2.17 vendor driver and checked against the staging rtl8723bs.
> > 
> I have no objection to values. 
> 
> > 
> > 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.
> > 
> I will think a bit how to align these messed tables. 

I understand. I am gonna send v8 today, and do you think that v8 could
be merged, so driver lands in 7.4 release?

Best regards,
Luka Gejak