Re: rtw88: rtl8822cs AP mode not working

[Martin Blumenstingl <martin.blumenstingl@googlemail.com>]

[-- Attachment #1: Type: text/plain, Size: 3081 bytes --]

Hi Ping-Ke and Viacheslav,

On Mon, Jan 15, 2024 at 3:17 AM Ping-Ke Shih <pkshih@realtek.com> wrote:
[...]
> > > I don't have this kind of wifi cards, could you help to check if AP mode
> > > works in your side?
> > I'll check that in the next few days.
AP mode is also not working for me, I get the same problem as
Viacheslav reported.

> > Also I'm wondering where code enables beacons (is it
> > rtw_core_enable_beacon() or is there another relevant function?).
> > Knowing that would be helpful to analyze this further.
>
> The main function to get and set beacon template to firmware is
> rtw_fw_download_rsvd_page(). The basic concept is to put beacon frame via
> qsel=BCN to a special TX FIFO area called "reserve page", and then
> hardware/firmware will send beacon in interval of 100ms.
Thanks for the explanation - that helped me find a better starting point!

I didn't have much time in the past few days, but I have some findings
and questions:
1) I found the following comment/code in the downstream driver [0]:
/*
 * Disable Hw protection for a time which revserd for Hw sending beacon.
 * Fix download reserved page packet fail that access collision with
the protection time.
 */
val8 = rtw_read8(adapter, REG_BCN_CTRL_8822C);
restore[1] = val8;
val8 &= ~BIT_EN_BCN_FUNCTION_8822C;
val8 |= BIT_DIS_TSF_UDT_8822C;
rtw_write8(adapter, REG_BCN_CTRL_8822C, val8);

This is not part of the upstream rtw88 driver, so I made a patch and
attached it.
Unfortunately it doesn't fix the problem.

2) PCI is the only HCI which does not need the checksum in the
pkt_Info (USB and SDIO need the checksum).
The checksum is added by rtw_tx_fill_txdesc_checksum() which is only
called in usb.c and sdio.c.
My understanding is that for reserved pages we can have more than one
pkt_info in the buffer (my starting point for this thought is
rtw_fill_rsvd_page_desc() from fw.c).
In usb.c and sdio.c we're only calculating the checksum for the very
first pkt_info, not for any subsequent ones (I didn't even know that
it's possible to have more than one pkt_Info outside of RX and TX
aggregation).
However, it seems that the downstream code calculates the TX checksum
for *all* pkt_info in the buffer, see [1]
This code is missing from rtw88 at the moment. Since I didn't have
time I did not try to implement this yet.

3) Has anybody tried AP mode with rtw88 on a (supported) USB chipset?
If my thought (from #2) is correct then AP mode would show the same
problems there.

4) Viacheslav, I think you previously mentioned that you did a bit of
work with the downstream driver.
It would be awesome if you could also take a look at the rtw88 and
downstream driver code and start comparing them (logic that's
different or completely missing from rtw88 is suspicious).


Best regards,
Martin


[0] https://github.com/chewitt/RTL8822CS/blob/60cd82134d63aa9436b43c42933a86d6e5a191ba/hal/rtl8822c/rtl8822c_ops.c#L1885-L1893
[1] https://github.com/chewitt/RTL8822CS/blob/main/hal/rtl8822c/sdio/rtl8822cs_xmit.c#L311-L312

[-- Attachment #2: rtw_fw_write_data_rsvd_page-write-REG_FWHW_TXQ_CTRL_2.diff --]
[-- Type: text/x-patch, Size: 1430 bytes --]

diff --git a/drivers/net/wireless/realtek/rtw88/fw.c b/drivers/net/wireless/realtek/rtw88/fw.c
index 3f037ddcecf1..22d2665b9f58 100644
--- a/drivers/net/wireless/realtek/rtw88/fw.c
+++ b/drivers/net/wireless/realtek/rtw88/fw.c
@@ -1437,7 +1437,7 @@ void rtw_add_rsvd_page_sta(struct rtw_dev *rtwdev,
 int rtw_fw_write_data_rsvd_page(struct rtw_dev *rtwdev, u16 pg_addr,
 				u8 *buf, u32 size)
 {
-	u8 bckp[2];
+	u8 bckp[3];
 	u8 val;
 	u16 rsvd_pg_head;
 	u32 bcn_valid_addr;
@@ -1462,8 +1462,14 @@ int rtw_fw_write_data_rsvd_page(struct rtw_dev *rtwdev, u16 pg_addr,
 	val |= BIT_ENSWBCN >> 8;
 	rtw_write8(rtwdev, REG_CR + 1, val);
 
-	val = rtw_read8(rtwdev, REG_FWHW_TXQ_CTRL + 2);
+	val = rtw_read8(rtwdev, REG_BCN_CTRL);
 	bckp[1] = val;
+	val &= ~BIT_EN_BCN_FUNCTION;
+	val |= BIT_DIS_TSF_UDT;
+	rtw_write8(rtwdev, REG_BCN_CTRL, val);
+
+	val = rtw_read8(rtwdev, REG_FWHW_TXQ_CTRL + 2);
+	bckp[2] = val;
 	val &= ~(BIT_EN_BCNQ_DL >> 16);
 	rtw_write8(rtwdev, REG_FWHW_TXQ_CTRL + 2, val);
 
@@ -1490,7 +1496,8 @@ int rtw_fw_write_data_rsvd_page(struct rtw_dev *rtwdev, u16 pg_addr,
 	rsvd_pg_head = rtwdev->fifo.rsvd_boundary;
 	rtw_write16(rtwdev, REG_FIFOPAGE_CTRL_2,
 		    rsvd_pg_head | BIT_BCN_VALID_V1);
-	rtw_write8(rtwdev, REG_FWHW_TXQ_CTRL + 2, bckp[1]);
+	rtw_write8(rtwdev, REG_FWHW_TXQ_CTRL + 2, bckp[2]);
+	rtw_write8(rtwdev, REG_BCN_CTRL, bckp[1]);
 	rtw_write8(rtwdev, REG_CR + 1, bckp[0]);
 
 	return ret;