linux-mmc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH wireless-next v2 0/4] wifi: rtw88: Add support for the RTL8723DS SDIO wifi chip
@ 2023-05-22 20:24 Martin Blumenstingl
  2023-05-22 20:24 ` [PATCH wireless-next v2 1/4] wifi: rtw88: sdio: Check the HISR RX_REQUEST bit in rtw_sdio_rx_isr() Martin Blumenstingl
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Martin Blumenstingl @ 2023-05-22 20:24 UTC (permalink / raw)
  To: linux-wireless
  Cc: linux-mmc, linux-kernel, ulf.hansson, kvalo, tony0620emma,
	Peter Robinson, Ping-Ke Shih, jernej.skrabec, Larry Finger,
	Martin Blumenstingl

This series adds support for the RTL823DS SDIO wifi chip.
It builds on the SDIO support which was recently merged into the rtw88
driver.

With the initial rtw88 SDIO support code the aim was to support these
older (802.11n) chips as well but we didn't have any way to test that.
Overall we spotted all corner cases in the vendor driver except one:
On these older chips REG_SDIO_RX0_REQ_LEN should only be read when
REG_SDIO_HISR has the REG_SDIO_HISR_RX_REQUEST bit set. Patch 1 adds
support for that.

RTL8723DS comes in two variant and each of them has their own SDIO ID:
- 0xd723 can connect two antennas. The WiFi part is still 1x1 so the
  second antenna can be dedicated to Bluetooth
- 0xd724 can only connect one antenna so it's shared between WiFi and
  Bluetooth
Thanks to Ping-Ke for these insights!

This series adds the missing SDIO ID and renames the existing 0xd723
SDIO ID (which we previously added) with patch 3. The rest is straight
forward: patch 2 adds support for parsing the eFuse and patch 4 wires
everything together by creating a driver for the RTL8723DS.

In my own tests on a MangoPi MQ-Quad I get the following results in
iperf3 (with the device being better placed than my RTL8822CS):
- RX: 48 Mbit/s
- TX: 33 Mbit/s
Also Larry Finger reports [0] that he has a user (with the rtw88
driver changes backported to support older kernels) that has
successfully tested RTL8723DS support.


Changes since v1 at [1]:
- rebased on top of 6.4-rc3 which comes with commit cb0ddaaa5db0
  ("wifi: rtw88: sdio: Always use two consecutive bytes for word
    operations")
- added braces to the new if condition in patch #1 as suggested by
  Ping-Ke (no functional changes but it makes that code block with
  the long comment easier to read/understand)
- collected Ping-Ke's Reviewed-by (thank you!)


[0] https://lore.kernel.org/linux-wireless/88e0c4a3-eec7-e44d-6f95-6f2e7f7cbbb5@lwfinger.net/
[1] https://lore.kernel.org/linux-wireless/20230518161749.1311949-1-martin.blumenstingl@googlemail.com/


Martin Blumenstingl (4):
  wifi: rtw88: sdio: Check the HISR RX_REQUEST bit in rtw_sdio_rx_isr()
  wifi: rtw88: rtw8723d: Implement RTL8723DS (SDIO) efuse parsing
  mmc: sdio: Add/rename SDIO ID of the RTL8723DS SDIO wifi cards
  wifi: rtw88: Add support for the SDIO based RTL8723DS chipset

 drivers/net/wireless/realtek/rtw88/Kconfig    | 11 +++++
 drivers/net/wireless/realtek/rtw88/Makefile   |  3 ++
 drivers/net/wireless/realtek/rtw88/rtw8723d.c |  9 ++++
 drivers/net/wireless/realtek/rtw88/rtw8723d.h |  6 +++
 .../net/wireless/realtek/rtw88/rtw8723ds.c    | 41 +++++++++++++++++++
 drivers/net/wireless/realtek/rtw88/sdio.c     | 24 +++++++++--
 include/linux/mmc/sdio_ids.h                  |  3 +-
 7 files changed, 93 insertions(+), 4 deletions(-)
 create mode 100644 drivers/net/wireless/realtek/rtw88/rtw8723ds.c

-- 
2.40.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH wireless-next v2 1/4] wifi: rtw88: sdio: Check the HISR RX_REQUEST bit in rtw_sdio_rx_isr()
  2023-05-22 20:24 [PATCH wireless-next v2 0/4] wifi: rtw88: Add support for the RTL8723DS SDIO wifi chip Martin Blumenstingl
@ 2023-05-22 20:24 ` Martin Blumenstingl
  2023-05-25 16:10   ` Kalle Valo
  2023-05-22 20:24 ` [PATCH wireless-next v2 2/4] wifi: rtw88: rtw8723d: Implement RTL8723DS (SDIO) efuse parsing Martin Blumenstingl
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Martin Blumenstingl @ 2023-05-22 20:24 UTC (permalink / raw)
  To: linux-wireless
  Cc: linux-mmc, linux-kernel, ulf.hansson, kvalo, tony0620emma,
	Peter Robinson, Ping-Ke Shih, jernej.skrabec, Larry Finger,
	Martin Blumenstingl

rtw_sdio_rx_isr() is responsible for receiving data from the wifi chip
and is called from the SDIO interrupt handler when the interrupt status
register (HISR) has the RX_REQUEST bit set. After the first batch of
data has been processed by the driver the wifi chip may have more data
ready to be read, which is managed by a loop in rtw_sdio_rx_isr().

It turns out that there are cases where the RX buffer length (from the
REG_SDIO_RX0_REQ_LEN register) does not match the data we receive. The
following two cases were observed with a RTL8723DS card:
- RX length is smaller than the total packet length including overhead
  and actual data bytes (whose length is part of the buffer we read from
  the wifi chip and is stored in rtw_rx_pkt_stat.pkt_len). This can
  result in errors like:
    skbuff: skb_over_panic: text:ffff8000011924ac len:3341 put:3341
  (one case observed was: RX buffer length = 1536 bytes but
   rtw_rx_pkt_stat.pkt_len = 1546 bytes, this is not valid as it means
   we need to read beyond the end of the buffer)
- RX length looks valid but rtw_rx_pkt_stat.pkt_len is zero

Check if the RX_REQUEST is set in the HISR register for each iteration
inside rtw_sdio_rx_isr(). This mimics what the RTL8723DS vendor driver
does and makes the driver only read more data if the RX_REQUEST bit is
set (which seems to be a way for the card's hardware or firmware to
tell the host that data is ready to be processed).

For RTW_WCPU_11AC chips this check is not needed. The RTL8822BS vendor
driver for example states that this check is unnecessary (but still uses
it) and the RTL8822CS drops this check entirely.

Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
Changes since v1:
- added Ping-Ke's Reviewed-by (thank you!)


 drivers/net/wireless/realtek/rtw88/sdio.c | 24 ++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/sdio.c b/drivers/net/wireless/realtek/rtw88/sdio.c
index 06fce7c3adda..2c1fb2dabd40 100644
--- a/drivers/net/wireless/realtek/rtw88/sdio.c
+++ b/drivers/net/wireless/realtek/rtw88/sdio.c
@@ -998,9 +998,9 @@ static void rtw_sdio_rxfifo_recv(struct rtw_dev *rtwdev, u32 rx_len)
 
 static void rtw_sdio_rx_isr(struct rtw_dev *rtwdev)
 {
-	u32 rx_len, total_rx_bytes = 0;
+	u32 rx_len, hisr, total_rx_bytes = 0;
 
-	while (total_rx_bytes < SZ_64K) {
+	do {
 		if (rtw_chip_wcpu_11n(rtwdev))
 			rx_len = rtw_read16(rtwdev, REG_SDIO_RX0_REQ_LEN);
 		else
@@ -1012,7 +1012,25 @@ static void rtw_sdio_rx_isr(struct rtw_dev *rtwdev)
 		rtw_sdio_rxfifo_recv(rtwdev, rx_len);
 
 		total_rx_bytes += rx_len;
-	}
+
+		if (rtw_chip_wcpu_11n(rtwdev)) {
+			/* Stop if no more RX requests are pending, even if
+			 * rx_len could be greater than zero in the next
+			 * iteration. This is needed because the RX buffer may
+			 * already contain data while either HW or FW are not
+			 * done filling that buffer yet. Still reading the
+			 * buffer can result in packets where
+			 * rtw_rx_pkt_stat.pkt_len is zero or points beyond the
+			 * end of the buffer.
+			 */
+			hisr = rtw_read32(rtwdev, REG_SDIO_HISR);
+		} else {
+			/* RTW_WCPU_11AC chips have improved hardware or
+			 * firmware and can use rx_len unconditionally.
+			 */
+			hisr = REG_SDIO_HISR_RX_REQUEST;
+		}
+	} while (total_rx_bytes < SZ_64K && hisr & REG_SDIO_HISR_RX_REQUEST);
 }
 
 static void rtw_sdio_handle_interrupt(struct sdio_func *sdio_func)
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH wireless-next v2 2/4] wifi: rtw88: rtw8723d: Implement RTL8723DS (SDIO) efuse parsing
  2023-05-22 20:24 [PATCH wireless-next v2 0/4] wifi: rtw88: Add support for the RTL8723DS SDIO wifi chip Martin Blumenstingl
  2023-05-22 20:24 ` [PATCH wireless-next v2 1/4] wifi: rtw88: sdio: Check the HISR RX_REQUEST bit in rtw_sdio_rx_isr() Martin Blumenstingl
@ 2023-05-22 20:24 ` Martin Blumenstingl
  2023-05-22 20:24 ` [PATCH wireless-next v2 3/4] mmc: sdio: Add/rename SDIO ID of the RTL8723DS SDIO wifi cards Martin Blumenstingl
  2023-05-22 20:24 ` [PATCH wireless-next v2 4/4] wifi: rtw88: Add support for the SDIO based RTL8723DS chipset Martin Blumenstingl
  3 siblings, 0 replies; 8+ messages in thread
From: Martin Blumenstingl @ 2023-05-22 20:24 UTC (permalink / raw)
  To: linux-wireless
  Cc: linux-mmc, linux-kernel, ulf.hansson, kvalo, tony0620emma,
	Peter Robinson, Ping-Ke Shih, jernej.skrabec, Larry Finger,
	Martin Blumenstingl

The efuse of the SDIO RTL8723DS chip has only one known member: the mac
address is at offset 0x11a. Add a struct rtw8723ds_efuse describing this
and use it for copying the mac address when the SDIO bus is used.

Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
Changes since v1:
- added Ping-Ke's Reviewed-by (thank you!)


 drivers/net/wireless/realtek/rtw88/rtw8723d.c | 9 +++++++++
 drivers/net/wireless/realtek/rtw88/rtw8723d.h | 6 ++++++
 2 files changed, 15 insertions(+)

diff --git a/drivers/net/wireless/realtek/rtw88/rtw8723d.c b/drivers/net/wireless/realtek/rtw88/rtw8723d.c
index 06e7454c9ca6..cadf66f4e854 100644
--- a/drivers/net/wireless/realtek/rtw88/rtw8723d.c
+++ b/drivers/net/wireless/realtek/rtw88/rtw8723d.c
@@ -216,6 +216,12 @@ static void rtw8723du_efuse_parsing(struct rtw_efuse *efuse,
 	ether_addr_copy(efuse->addr, map->u.mac_addr);
 }
 
+static void rtw8723ds_efuse_parsing(struct rtw_efuse *efuse,
+				    struct rtw8723d_efuse *map)
+{
+	ether_addr_copy(efuse->addr, map->s.mac_addr);
+}
+
 static int rtw8723d_read_efuse(struct rtw_dev *rtwdev, u8 *log_map)
 {
 	struct rtw_efuse *efuse = &rtwdev->efuse;
@@ -248,6 +254,9 @@ static int rtw8723d_read_efuse(struct rtw_dev *rtwdev, u8 *log_map)
 	case RTW_HCI_TYPE_USB:
 		rtw8723du_efuse_parsing(efuse, map);
 		break;
+	case RTW_HCI_TYPE_SDIO:
+		rtw8723ds_efuse_parsing(efuse, map);
+		break;
 	default:
 		/* unsupported now */
 		return -ENOTSUPP;
diff --git a/drivers/net/wireless/realtek/rtw88/rtw8723d.h b/drivers/net/wireless/realtek/rtw88/rtw8723d.h
index a356318a5c15..3642a2c7f80c 100644
--- a/drivers/net/wireless/realtek/rtw88/rtw8723d.h
+++ b/drivers/net/wireless/realtek/rtw88/rtw8723d.h
@@ -49,6 +49,11 @@ struct rtw8723du_efuse {
 	u8 mac_addr[ETH_ALEN];          /* 0x107 */
 };
 
+struct rtw8723ds_efuse {
+	u8 res4[0x4a];			/* 0xd0 */
+	u8 mac_addr[ETH_ALEN];		/* 0x11a */
+};
+
 struct rtw8723d_efuse {
 	__le16 rtl_id;
 	u8 rsvd[2];
@@ -80,6 +85,7 @@ struct rtw8723d_efuse {
 	union {
 		struct rtw8723de_efuse e;
 		struct rtw8723du_efuse u;
+		struct rtw8723ds_efuse s;
 	};
 };
 
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH wireless-next v2 3/4] mmc: sdio: Add/rename SDIO ID of the RTL8723DS SDIO wifi cards
  2023-05-22 20:24 [PATCH wireless-next v2 0/4] wifi: rtw88: Add support for the RTL8723DS SDIO wifi chip Martin Blumenstingl
  2023-05-22 20:24 ` [PATCH wireless-next v2 1/4] wifi: rtw88: sdio: Check the HISR RX_REQUEST bit in rtw_sdio_rx_isr() Martin Blumenstingl
  2023-05-22 20:24 ` [PATCH wireless-next v2 2/4] wifi: rtw88: rtw8723d: Implement RTL8723DS (SDIO) efuse parsing Martin Blumenstingl
@ 2023-05-22 20:24 ` Martin Blumenstingl
  2023-05-22 20:24 ` [PATCH wireless-next v2 4/4] wifi: rtw88: Add support for the SDIO based RTL8723DS chipset Martin Blumenstingl
  3 siblings, 0 replies; 8+ messages in thread
From: Martin Blumenstingl @ 2023-05-22 20:24 UTC (permalink / raw)
  To: linux-wireless
  Cc: linux-mmc, linux-kernel, ulf.hansson, kvalo, tony0620emma,
	Peter Robinson, Ping-Ke Shih, jernej.skrabec, Larry Finger,
	Martin Blumenstingl

RTL8723DS comes in two variant and each of them has their own SDIO ID:
- 0xd723 can connect two antennas. The WiFi part is still 1x1 so the
  second antenna can be dedicated to Bluetooth
- 0xd724 can only connect one antenna so it's shared between WiFi and
  Bluetooth

Add a new entry for the single antenna RTL8723DS (0xd724) which can be
found on the MangoPi MQ-Quad. Also rename the existing RTL8723DS entry
(0xd723) so it's name reflects that it's the variant with support for
two antennas.

Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
Changes since v1:
- added Ping-Ke's Reviewed-by (thank you!)


 include/linux/mmc/sdio_ids.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/linux/mmc/sdio_ids.h b/include/linux/mmc/sdio_ids.h
index c653accdc7fd..7fada7a714fe 100644
--- a/include/linux/mmc/sdio_ids.h
+++ b/include/linux/mmc/sdio_ids.h
@@ -121,7 +121,8 @@
 #define SDIO_DEVICE_ID_REALTEK_RTW8822BS	0xb822
 #define SDIO_DEVICE_ID_REALTEK_RTW8821CS	0xc821
 #define SDIO_DEVICE_ID_REALTEK_RTW8822CS	0xc822
-#define SDIO_DEVICE_ID_REALTEK_RTW8723DS	0xd723
+#define SDIO_DEVICE_ID_REALTEK_RTW8723DS_2ANT	0xd723
+#define SDIO_DEVICE_ID_REALTEK_RTW8723DS_1ANT	0xd724
 #define SDIO_DEVICE_ID_REALTEK_RTW8821DS	0xd821
 
 #define SDIO_VENDOR_ID_SIANO			0x039a
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH wireless-next v2 4/4] wifi: rtw88: Add support for the SDIO based RTL8723DS chipset
  2023-05-22 20:24 [PATCH wireless-next v2 0/4] wifi: rtw88: Add support for the RTL8723DS SDIO wifi chip Martin Blumenstingl
                   ` (2 preceding siblings ...)
  2023-05-22 20:24 ` [PATCH wireless-next v2 3/4] mmc: sdio: Add/rename SDIO ID of the RTL8723DS SDIO wifi cards Martin Blumenstingl
@ 2023-05-22 20:24 ` Martin Blumenstingl
  3 siblings, 0 replies; 8+ messages in thread
From: Martin Blumenstingl @ 2023-05-22 20:24 UTC (permalink / raw)
  To: linux-wireless
  Cc: linux-mmc, linux-kernel, ulf.hansson, kvalo, tony0620emma,
	Peter Robinson, Ping-Ke Shih, jernej.skrabec, Larry Finger,
	Martin Blumenstingl

Wire up RTL8723DS chipset support using the rtw88 SDIO HCI code as well
as the existing RTL8723D chipset code.

Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
Changes since v1:
- added Ping-Ke's Reviewed-by (thank you!)


 drivers/net/wireless/realtek/rtw88/Kconfig    | 11 +++++
 drivers/net/wireless/realtek/rtw88/Makefile   |  3 ++
 .../net/wireless/realtek/rtw88/rtw8723ds.c    | 41 +++++++++++++++++++
 3 files changed, 55 insertions(+)
 create mode 100644 drivers/net/wireless/realtek/rtw88/rtw8723ds.c

diff --git a/drivers/net/wireless/realtek/rtw88/Kconfig b/drivers/net/wireless/realtek/rtw88/Kconfig
index 29eb2f8e0eb7..cffad1c01249 100644
--- a/drivers/net/wireless/realtek/rtw88/Kconfig
+++ b/drivers/net/wireless/realtek/rtw88/Kconfig
@@ -111,6 +111,17 @@ config RTW88_8723DE
 
 	  802.11n PCIe wireless network adapter
 
+config RTW88_8723DS
+	tristate "Realtek 8723DS SDIO wireless network adapter"
+	depends on MMC
+	select RTW88_CORE
+	select RTW88_SDIO
+	select RTW88_8723D
+	help
+	  Select this option will enable support for 8723DS chipset
+
+	  802.11n SDIO wireless network adapter
+
 config RTW88_8723DU
 	tristate "Realtek 8723DU USB wireless network adapter"
 	depends on USB
diff --git a/drivers/net/wireless/realtek/rtw88/Makefile b/drivers/net/wireless/realtek/rtw88/Makefile
index 82979b30ae8d..fd212c09d88a 100644
--- a/drivers/net/wireless/realtek/rtw88/Makefile
+++ b/drivers/net/wireless/realtek/rtw88/Makefile
@@ -50,6 +50,9 @@ rtw88_8723d-objs		:= rtw8723d.o rtw8723d_table.o
 obj-$(CONFIG_RTW88_8723DE)	+= rtw88_8723de.o
 rtw88_8723de-objs		:= rtw8723de.o
 
+obj-$(CONFIG_RTW88_8723DS)	+= rtw88_8723ds.o
+rtw88_8723ds-objs		:= rtw8723ds.o
+
 obj-$(CONFIG_RTW88_8723DU)	+= rtw88_8723du.o
 rtw88_8723du-objs		:= rtw8723du.o
 
diff --git a/drivers/net/wireless/realtek/rtw88/rtw8723ds.c b/drivers/net/wireless/realtek/rtw88/rtw8723ds.c
new file mode 100644
index 000000000000..e5b6960ba0a0
--- /dev/null
+++ b/drivers/net/wireless/realtek/rtw88/rtw8723ds.c
@@ -0,0 +1,41 @@
+// SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause
+/* Copyright(c) Martin Blumenstingl <martin.blumenstingl@googlemail.com>
+ */
+
+#include <linux/mmc/sdio_func.h>
+#include <linux/mmc/sdio_ids.h>
+#include <linux/module.h>
+#include "main.h"
+#include "rtw8723d.h"
+#include "sdio.h"
+
+static const struct sdio_device_id rtw_8723ds_id_table[] =  {
+	{
+		SDIO_DEVICE(SDIO_VENDOR_ID_REALTEK,
+			    SDIO_DEVICE_ID_REALTEK_RTW8723DS_1ANT),
+		.driver_data = (kernel_ulong_t)&rtw8723d_hw_spec,
+	},
+	{
+		SDIO_DEVICE(SDIO_VENDOR_ID_REALTEK,
+			    SDIO_DEVICE_ID_REALTEK_RTW8723DS_2ANT),
+		.driver_data = (kernel_ulong_t)&rtw8723d_hw_spec,
+	},
+	{}
+};
+MODULE_DEVICE_TABLE(sdio, rtw_8723ds_id_table);
+
+static struct sdio_driver rtw_8723ds_driver = {
+	.name = "rtw_8723ds",
+	.probe = rtw_sdio_probe,
+	.remove = rtw_sdio_remove,
+	.id_table = rtw_8723ds_id_table,
+	.drv = {
+		.pm = &rtw_sdio_pm_ops,
+		.shutdown = rtw_sdio_shutdown,
+	}
+};
+module_sdio_driver(rtw_8723ds_driver);
+
+MODULE_AUTHOR("Martin Blumenstingl <martin.blumenstingl@googlemail.com>");
+MODULE_DESCRIPTION("Realtek 802.11n wireless 8723ds driver");
+MODULE_LICENSE("Dual BSD/GPL");
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH wireless-next v2 1/4] wifi: rtw88: sdio: Check the HISR RX_REQUEST bit in rtw_sdio_rx_isr()
  2023-05-22 20:24 ` [PATCH wireless-next v2 1/4] wifi: rtw88: sdio: Check the HISR RX_REQUEST bit in rtw_sdio_rx_isr() Martin Blumenstingl
@ 2023-05-25 16:10   ` Kalle Valo
  2023-05-26 13:11     ` Ulf Hansson
  0 siblings, 1 reply; 8+ messages in thread
From: Kalle Valo @ 2023-05-25 16:10 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: linux-wireless, linux-mmc, linux-kernel, ulf.hansson,
	tony0620emma, Peter Robinson, Ping-Ke Shih, jernej.skrabec,
	Larry Finger, Martin Blumenstingl

Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote:

> rtw_sdio_rx_isr() is responsible for receiving data from the wifi chip
> and is called from the SDIO interrupt handler when the interrupt status
> register (HISR) has the RX_REQUEST bit set. After the first batch of
> data has been processed by the driver the wifi chip may have more data
> ready to be read, which is managed by a loop in rtw_sdio_rx_isr().
> 
> It turns out that there are cases where the RX buffer length (from the
> REG_SDIO_RX0_REQ_LEN register) does not match the data we receive. The
> following two cases were observed with a RTL8723DS card:
> - RX length is smaller than the total packet length including overhead
>   and actual data bytes (whose length is part of the buffer we read from
>   the wifi chip and is stored in rtw_rx_pkt_stat.pkt_len). This can
>   result in errors like:
>     skbuff: skb_over_panic: text:ffff8000011924ac len:3341 put:3341
>   (one case observed was: RX buffer length = 1536 bytes but
>    rtw_rx_pkt_stat.pkt_len = 1546 bytes, this is not valid as it means
>    we need to read beyond the end of the buffer)
> - RX length looks valid but rtw_rx_pkt_stat.pkt_len is zero
> 
> Check if the RX_REQUEST is set in the HISR register for each iteration
> inside rtw_sdio_rx_isr(). This mimics what the RTL8723DS vendor driver
> does and makes the driver only read more data if the RX_REQUEST bit is
> set (which seems to be a way for the card's hardware or firmware to
> tell the host that data is ready to be processed).
> 
> For RTW_WCPU_11AC chips this check is not needed. The RTL8822BS vendor
> driver for example states that this check is unnecessary (but still uses
> it) and the RTL8822CS drops this check entirely.
> 
> Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

4 patches applied to wireless-next.git, thanks.

e967229ead0e wifi: rtw88: sdio: Check the HISR RX_REQUEST bit in rtw_sdio_rx_isr()
9be20a822327 wifi: rtw88: rtw8723d: Implement RTL8723DS (SDIO) efuse parsing
09fcdbd28404 mmc: sdio: Add/rename SDIO ID of the RTL8723DS SDIO wifi cards
a3b125ceb45e wifi: rtw88: Add support for the SDIO based RTL8723DS chipset

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/20230522202425.1827005-2-martin.blumenstingl@googlemail.com/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH wireless-next v2 1/4] wifi: rtw88: sdio: Check the HISR RX_REQUEST bit in rtw_sdio_rx_isr()
  2023-05-25 16:10   ` Kalle Valo
@ 2023-05-26 13:11     ` Ulf Hansson
  2023-05-26 14:14       ` Kalle Valo
  0 siblings, 1 reply; 8+ messages in thread
From: Ulf Hansson @ 2023-05-26 13:11 UTC (permalink / raw)
  To: Kalle Valo
  Cc: Martin Blumenstingl, linux-wireless, linux-mmc, linux-kernel,
	tony0620emma, Peter Robinson, Ping-Ke Shih, jernej.skrabec,
	Larry Finger

On Thu, 25 May 2023 at 18:10, Kalle Valo <kvalo@kernel.org> wrote:
>
> Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote:
>
> > rtw_sdio_rx_isr() is responsible for receiving data from the wifi chip
> > and is called from the SDIO interrupt handler when the interrupt status
> > register (HISR) has the RX_REQUEST bit set. After the first batch of
> > data has been processed by the driver the wifi chip may have more data
> > ready to be read, which is managed by a loop in rtw_sdio_rx_isr().
> >
> > It turns out that there are cases where the RX buffer length (from the
> > REG_SDIO_RX0_REQ_LEN register) does not match the data we receive. The
> > following two cases were observed with a RTL8723DS card:
> > - RX length is smaller than the total packet length including overhead
> >   and actual data bytes (whose length is part of the buffer we read from
> >   the wifi chip and is stored in rtw_rx_pkt_stat.pkt_len). This can
> >   result in errors like:
> >     skbuff: skb_over_panic: text:ffff8000011924ac len:3341 put:3341
> >   (one case observed was: RX buffer length = 1536 bytes but
> >    rtw_rx_pkt_stat.pkt_len = 1546 bytes, this is not valid as it means
> >    we need to read beyond the end of the buffer)
> > - RX length looks valid but rtw_rx_pkt_stat.pkt_len is zero
> >
> > Check if the RX_REQUEST is set in the HISR register for each iteration
> > inside rtw_sdio_rx_isr(). This mimics what the RTL8723DS vendor driver
> > does and makes the driver only read more data if the RX_REQUEST bit is
> > set (which seems to be a way for the card's hardware or firmware to
> > tell the host that data is ready to be processed).
> >
> > For RTW_WCPU_11AC chips this check is not needed. The RTL8822BS vendor
> > driver for example states that this check is unnecessary (but still uses
> > it) and the RTL8822CS drops this check entirely.
> >
> > Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>
> > Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>
> 4 patches applied to wireless-next.git, thanks.
>
> e967229ead0e wifi: rtw88: sdio: Check the HISR RX_REQUEST bit in rtw_sdio_rx_isr()
> 9be20a822327 wifi: rtw88: rtw8723d: Implement RTL8723DS (SDIO) efuse parsing
> 09fcdbd28404 mmc: sdio: Add/rename SDIO ID of the RTL8723DS SDIO wifi cards
> a3b125ceb45e wifi: rtw88: Add support for the SDIO based RTL8723DS chipset

If it's not too late, feel free to add my ack for patch3. Nevermind,
if it adds additional work for you.

Kind regards
Uffe

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH wireless-next v2 1/4] wifi: rtw88: sdio: Check the HISR RX_REQUEST bit in rtw_sdio_rx_isr()
  2023-05-26 13:11     ` Ulf Hansson
@ 2023-05-26 14:14       ` Kalle Valo
  0 siblings, 0 replies; 8+ messages in thread
From: Kalle Valo @ 2023-05-26 14:14 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Martin Blumenstingl, linux-wireless, linux-mmc, linux-kernel,
	tony0620emma, Peter Robinson, Ping-Ke Shih, jernej.skrabec,
	Larry Finger

Ulf Hansson <ulf.hansson@linaro.org> writes:

> On Thu, 25 May 2023 at 18:10, Kalle Valo <kvalo@kernel.org> wrote:
>
>>
>> Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote:
>>
>> > rtw_sdio_rx_isr() is responsible for receiving data from the wifi chip
>> > and is called from the SDIO interrupt handler when the interrupt status
>> > register (HISR) has the RX_REQUEST bit set. After the first batch of
>> > data has been processed by the driver the wifi chip may have more data
>> > ready to be read, which is managed by a loop in rtw_sdio_rx_isr().
>> >
>> > It turns out that there are cases where the RX buffer length (from the
>> > REG_SDIO_RX0_REQ_LEN register) does not match the data we receive. The
>> > following two cases were observed with a RTL8723DS card:
>> > - RX length is smaller than the total packet length including overhead
>> >   and actual data bytes (whose length is part of the buffer we read from
>> >   the wifi chip and is stored in rtw_rx_pkt_stat.pkt_len). This can
>> >   result in errors like:
>> >     skbuff: skb_over_panic: text:ffff8000011924ac len:3341 put:3341
>> >   (one case observed was: RX buffer length = 1536 bytes but
>> >    rtw_rx_pkt_stat.pkt_len = 1546 bytes, this is not valid as it means
>> >    we need to read beyond the end of the buffer)
>> > - RX length looks valid but rtw_rx_pkt_stat.pkt_len is zero
>> >
>> > Check if the RX_REQUEST is set in the HISR register for each iteration
>> > inside rtw_sdio_rx_isr(). This mimics what the RTL8723DS vendor driver
>> > does and makes the driver only read more data if the RX_REQUEST bit is
>> > set (which seems to be a way for the card's hardware or firmware to
>> > tell the host that data is ready to be processed).
>> >
>> > For RTW_WCPU_11AC chips this check is not needed. The RTL8822BS vendor
>> > driver for example states that this check is unnecessary (but still uses
>> > it) and the RTL8822CS drops this check entirely.
>> >
>> > Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>
>> > Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>>
>> 4 patches applied to wireless-next.git, thanks.
>>
>> e967229ead0e wifi: rtw88: sdio: Check the HISR RX_REQUEST bit in rtw_sdio_rx_isr()
>> 9be20a822327 wifi: rtw88: rtw8723d: Implement RTL8723DS (SDIO) efuse parsing
>> 09fcdbd28404 mmc: sdio: Add/rename SDIO ID of the RTL8723DS SDIO wifi cards
>> a3b125ceb45e wifi: rtw88: Add support for the SDIO based RTL8723DS chipset
>
> If it's not too late, feel free to add my ack for patch3. Nevermind,
> if it adds additional work for you.

We don't rebase wireless trees so I can't add it anymore, sorry. But
thanks for reviewing it.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2023-05-26 14:14 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-22 20:24 [PATCH wireless-next v2 0/4] wifi: rtw88: Add support for the RTL8723DS SDIO wifi chip Martin Blumenstingl
2023-05-22 20:24 ` [PATCH wireless-next v2 1/4] wifi: rtw88: sdio: Check the HISR RX_REQUEST bit in rtw_sdio_rx_isr() Martin Blumenstingl
2023-05-25 16:10   ` Kalle Valo
2023-05-26 13:11     ` Ulf Hansson
2023-05-26 14:14       ` Kalle Valo
2023-05-22 20:24 ` [PATCH wireless-next v2 2/4] wifi: rtw88: rtw8723d: Implement RTL8723DS (SDIO) efuse parsing Martin Blumenstingl
2023-05-22 20:24 ` [PATCH wireless-next v2 3/4] mmc: sdio: Add/rename SDIO ID of the RTL8723DS SDIO wifi cards Martin Blumenstingl
2023-05-22 20:24 ` [PATCH wireless-next v2 4/4] wifi: rtw88: Add support for the SDIO based RTL8723DS chipset Martin Blumenstingl

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).