From dfec271b3a5b13b133c962ac3a4e61405502f610 Mon Sep 17 00:00:00 2001 From: Patrick Yavitz Date: Fri, 1 Dec 2023 07:59:26 -0500 Subject: [PATCH] v3: rtw88: sdio: Honor the host max_req_size in the RX path The 3rd revision of the patch has been applied upstream. https://lore.kernel.org/linux-wireless/87cyvqsabo.fsf@kernel.org/T/#t Signed-off-by: Patrick Yavitz --- ...less-realtek-rtw88-upstream-wireless.patch | 76 ++++++++++++++----- ...less-realtek-rtw88-upstream-wireless.patch | 75 +++++++++++++----- ...less-realtek-rtw88-upstream-wireless.patch | 75 +++++++++++++----- 3 files changed, 171 insertions(+), 55 deletions(-) diff --git a/patch/misc/rtw88/6.1/001-drivers-net-wireless-realtek-rtw88-upstream-wireless.patch b/patch/misc/rtw88/6.1/001-drivers-net-wireless-realtek-rtw88-upstream-wireless.patch index d7347a8b7..f88dc0366 100644 --- a/patch/misc/rtw88/6.1/001-drivers-net-wireless-realtek-rtw88-upstream-wireless.patch +++ b/patch/misc/rtw88/6.1/001-drivers-net-wireless-realtek-rtw88-upstream-wireless.patch @@ -6161,12 +6161,10 @@ index 74f9d9a6d330..f30a4e564754 100644 -- 2.39.2 - From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Martin Blumenstingl -To: linux-wireless@vger.kernel.org -Date: Wed, 2 Aug 2023 00:27:32 +0000 -Subject: [PATCH] wifi: rtw88: sdio: Honor the host max_req_size in the RX path +Date: Mon, 20 Nov 2023 12:57:26 +0100 +Subject: [PATCH v3] wifi: rtw88: sdio: Honor the host max_req_size in the RX path Lukas reports skb_over_panic errors on his Banana Pi BPI-CM4 which comes with an Amlogic A311D (G12B) SoC and a RTL8822CS SDIO wifi/Bluetooth @@ -6174,9 +6172,10 @@ combo card. The error he observed is identical to what has been fixed in commit e967229ead0e ("wifi: rtw88: sdio: Check the HISR RX_REQUEST bit in rtw_sdio_rx_isr()") but that commit didn't fix Lukas' problem. -Lukas found that disabling or limiting RX aggregation fix the problem -for him. In the following discussion a few key topics have been -discussed which have an impact on this problem: +Lukas found that disabling or limiting RX aggregation works around the +problem for some time (but does not fully fix it). In the following +discussion a few key topics have been discussed which have an impact on +this problem: - The Amlogic A311D (G12B) SoC has a hardware bug in the SDIO controller which prevents DMA transfers. Instead all transfers need to go through the controller SRAM which limits transfers to 1536 bytes @@ -6189,7 +6188,7 @@ discussed which have an impact on this problem: be aggregated, BIT_DMA_AGG_TO_V1 configures a timeout for aggregation and BIT_EN_PRE_CALC makes the chip honor the limits more effectively) -Use multiple consecutive reads in rtw_sdio_read_port() to limit the +Use multiple consecutive reads in rtw_sdio_read_port() and limit the number of bytes which are copied by the host from the card in one MMC/SDIO transfer. This allows receiving a buffer that's larger than the hosts max_req_size (number of bytes which can be transferred in @@ -6198,28 +6197,58 @@ is gone as the rtw88 driver is now able to receive more than 1536 bytes from the card (either because the incoming packet is larger than that or because multiple packets have been aggregated). +In case of an receive errors (-EILSEQ has been observed by Lukas) we +need to drain the remaining data from the card's buffer, otherwise the +card will return corrupt data for the next rtw_sdio_read_port() call. + Fixes: 65371a3f14e7 ("wifi: rtw88: sdio: Add HCI implementation for SDIO based chipsets") Reported-by: Lukas F. Hartmann Closes: https://lore.kernel.org/linux-wireless/CAFBinCBaXtebixKbjkWKW_WXc5k=NdGNaGUjVE8NCPNxOhsb2g@mail.gmail.com/ Suggested-by: Ping-Ke Shih Signed-off-by: Martin Blumenstingl --- - drivers/net/wireless/realtek/rtw88/sdio.c | 24 +++++++++++++++++------ - 1 file changed, 18 insertions(+), 6 deletions(-) + +Changes since v2 at [2]: +- Don't initialize err to zero as that intiial value is never used. + Thanks Ping-Ke for spotting this! +- Add a comment explaning why we need to continue reading but still + have to return an error to the caller of rtw_sdio_read_port() + +Changes since v1 at [0]: +- We need to read all bytes if we split the transaction into multiple + smaller reads. This is even the case when one of N reads reports an + error. Otherwise the next read port call will return garbage (partially + containing zeros, ...). A similar-ish approach can be found in the + vendor driver, see [1] (specifically the call to sdio_recv_and_drop()) +- Update the patch description accordingly + +With a preliminary version of this updated patch Lukas reported off- +list: "i've been using this laptop for almost 3 hours with heavy wifi +usage and so far no problems" + + +[0] https://lore.kernel.org/lkml/169089906853.212423.17095176293160428610.kvalo@kernel.org/T/ +[1] https://github.com/chewitt/RTL8822CS/blob/ad1391e219b59314485739a499fb442d5bbc069e/hal/rtl8822c/sdio/rtl8822cs_io.c#L468-L477 +[2] https://lore.kernel.org/linux-wireless/20230806181656.2072792-1-martin.blumenstingl@googlemail.com/ + + + drivers/net/wireless/realtek/rtw88/sdio.c | 35 ++++++++++++++++++----- + 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/drivers/net/wireless/realtek/rtw88/sdio.c b/drivers/net/wireless/realtek/rtw88/sdio.c -index 2c1fb2dabd40..b19262ec5d8c 100644 +index 2c1fb2dabd40..0cae5746f540 100644 --- a/drivers/net/wireless/realtek/rtw88/sdio.c +++ b/drivers/net/wireless/realtek/rtw88/sdio.c -@@ -500,19 +500,31 @@ static u32 rtw_sdio_get_tx_addr(struct rtw_dev *rtwdev, size_t size, +@@ -500,19 +500,40 @@ static u32 rtw_sdio_get_tx_addr(struct rtw_dev *rtwdev, size_t size, static int rtw_sdio_read_port(struct rtw_dev *rtwdev, u8 *buf, size_t count) { struct rtw_sdio *rtwsdio = (struct rtw_sdio *)rtwdev->priv; + struct mmc_host *host = rtwsdio->sdio_func->card->host; bool bus_claim = rtw_sdio_bus_claim_needed(rtwsdio); u32 rxaddr = rtwsdio->rx_addr++; +- int ret; ++ int ret = 0, err; + size_t bytes; - int ret; if (bus_claim) sdio_claim_host(rtwsdio->sdio_func); @@ -6233,14 +6262,23 @@ index 2c1fb2dabd40..b19262ec5d8c 100644 + while (count > 0) { + bytes = min_t(size_t, host->max_req_size, count); + -+ ret = sdio_memcpy_fromio(rtwsdio->sdio_func, buf, ++ err = sdio_memcpy_fromio(rtwsdio->sdio_func, buf, + RTW_SDIO_ADDR_RX_RX0FF_GEN(rxaddr), + bytes); -+ if (ret) { ++ if (err) { + rtw_warn(rtwdev, -+ "Failed to read %zu byte(s) from SDIO port 0x%08x", -+ bytes, rxaddr); -+ break; ++ "Failed to read %zu byte(s) from SDIO port 0x%08x: %d", ++ bytes, rxaddr, err); ++ ++ /* Signal to the caller that reading did not work and ++ * that the data in the buffer is short/corrupted. ++ */ ++ ret = err; ++ ++ /* Don't stop here - instead drain the remaining data ++ * from the card's buffer, else the card will return ++ * corrupt data for the next rtw_sdio_read_port() call. ++ */ + } + + count -= bytes; @@ -6250,7 +6288,7 @@ index 2c1fb2dabd40..b19262ec5d8c 100644 if (bus_claim) sdio_release_host(rtwsdio->sdio_func); -- -2.41.0 +2.42.1 From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Ping-Ke Shih diff --git a/patch/misc/rtw88/6.6/001-drivers-net-wireless-realtek-rtw88-upstream-wireless.patch b/patch/misc/rtw88/6.6/001-drivers-net-wireless-realtek-rtw88-upstream-wireless.patch index 7bed46011..8d8edd901 100644 --- a/patch/misc/rtw88/6.6/001-drivers-net-wireless-realtek-rtw88-upstream-wireless.patch +++ b/patch/misc/rtw88/6.6/001-drivers-net-wireless-realtek-rtw88-upstream-wireless.patch @@ -1,8 +1,7 @@ From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Martin Blumenstingl -To: linux-wireless@vger.kernel.org -Date: Wed, 2 Aug 2023 00:27:32 +0000 -Subject: [PATCH] wifi: rtw88: sdio: Honor the host max_req_size in the RX path +Date: Mon, 20 Nov 2023 12:57:26 +0100 +Subject: [PATCH v3] wifi: rtw88: sdio: Honor the host max_req_size in the RX path Lukas reports skb_over_panic errors on his Banana Pi BPI-CM4 which comes with an Amlogic A311D (G12B) SoC and a RTL8822CS SDIO wifi/Bluetooth @@ -10,9 +9,10 @@ combo card. The error he observed is identical to what has been fixed in commit e967229ead0e ("wifi: rtw88: sdio: Check the HISR RX_REQUEST bit in rtw_sdio_rx_isr()") but that commit didn't fix Lukas' problem. -Lukas found that disabling or limiting RX aggregation fix the problem -for him. In the following discussion a few key topics have been -discussed which have an impact on this problem: +Lukas found that disabling or limiting RX aggregation works around the +problem for some time (but does not fully fix it). In the following +discussion a few key topics have been discussed which have an impact on +this problem: - The Amlogic A311D (G12B) SoC has a hardware bug in the SDIO controller which prevents DMA transfers. Instead all transfers need to go through the controller SRAM which limits transfers to 1536 bytes @@ -25,7 +25,7 @@ discussed which have an impact on this problem: be aggregated, BIT_DMA_AGG_TO_V1 configures a timeout for aggregation and BIT_EN_PRE_CALC makes the chip honor the limits more effectively) -Use multiple consecutive reads in rtw_sdio_read_port() to limit the +Use multiple consecutive reads in rtw_sdio_read_port() and limit the number of bytes which are copied by the host from the card in one MMC/SDIO transfer. This allows receiving a buffer that's larger than the hosts max_req_size (number of bytes which can be transferred in @@ -34,28 +34,58 @@ is gone as the rtw88 driver is now able to receive more than 1536 bytes from the card (either because the incoming packet is larger than that or because multiple packets have been aggregated). +In case of an receive errors (-EILSEQ has been observed by Lukas) we +need to drain the remaining data from the card's buffer, otherwise the +card will return corrupt data for the next rtw_sdio_read_port() call. + Fixes: 65371a3f14e7 ("wifi: rtw88: sdio: Add HCI implementation for SDIO based chipsets") Reported-by: Lukas F. Hartmann Closes: https://lore.kernel.org/linux-wireless/CAFBinCBaXtebixKbjkWKW_WXc5k=NdGNaGUjVE8NCPNxOhsb2g@mail.gmail.com/ Suggested-by: Ping-Ke Shih Signed-off-by: Martin Blumenstingl --- - drivers/net/wireless/realtek/rtw88/sdio.c | 24 +++++++++++++++++------ - 1 file changed, 18 insertions(+), 6 deletions(-) + +Changes since v2 at [2]: +- Don't initialize err to zero as that intiial value is never used. + Thanks Ping-Ke for spotting this! +- Add a comment explaning why we need to continue reading but still + have to return an error to the caller of rtw_sdio_read_port() + +Changes since v1 at [0]: +- We need to read all bytes if we split the transaction into multiple + smaller reads. This is even the case when one of N reads reports an + error. Otherwise the next read port call will return garbage (partially + containing zeros, ...). A similar-ish approach can be found in the + vendor driver, see [1] (specifically the call to sdio_recv_and_drop()) +- Update the patch description accordingly + +With a preliminary version of this updated patch Lukas reported off- +list: "i've been using this laptop for almost 3 hours with heavy wifi +usage and so far no problems" + + +[0] https://lore.kernel.org/lkml/169089906853.212423.17095176293160428610.kvalo@kernel.org/T/ +[1] https://github.com/chewitt/RTL8822CS/blob/ad1391e219b59314485739a499fb442d5bbc069e/hal/rtl8822c/sdio/rtl8822cs_io.c#L468-L477 +[2] https://lore.kernel.org/linux-wireless/20230806181656.2072792-1-martin.blumenstingl@googlemail.com/ + + + drivers/net/wireless/realtek/rtw88/sdio.c | 35 ++++++++++++++++++----- + 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/drivers/net/wireless/realtek/rtw88/sdio.c b/drivers/net/wireless/realtek/rtw88/sdio.c -index 2c1fb2dabd40..b19262ec5d8c 100644 +index 2c1fb2dabd40..0cae5746f540 100644 --- a/drivers/net/wireless/realtek/rtw88/sdio.c +++ b/drivers/net/wireless/realtek/rtw88/sdio.c -@@ -500,19 +500,31 @@ static u32 rtw_sdio_get_tx_addr(struct rtw_dev *rtwdev, size_t size, +@@ -500,19 +500,40 @@ static u32 rtw_sdio_get_tx_addr(struct rtw_dev *rtwdev, size_t size, static int rtw_sdio_read_port(struct rtw_dev *rtwdev, u8 *buf, size_t count) { struct rtw_sdio *rtwsdio = (struct rtw_sdio *)rtwdev->priv; + struct mmc_host *host = rtwsdio->sdio_func->card->host; bool bus_claim = rtw_sdio_bus_claim_needed(rtwsdio); u32 rxaddr = rtwsdio->rx_addr++; +- int ret; ++ int ret = 0, err; + size_t bytes; - int ret; if (bus_claim) sdio_claim_host(rtwsdio->sdio_func); @@ -69,14 +99,23 @@ index 2c1fb2dabd40..b19262ec5d8c 100644 + while (count > 0) { + bytes = min_t(size_t, host->max_req_size, count); + -+ ret = sdio_memcpy_fromio(rtwsdio->sdio_func, buf, ++ err = sdio_memcpy_fromio(rtwsdio->sdio_func, buf, + RTW_SDIO_ADDR_RX_RX0FF_GEN(rxaddr), + bytes); -+ if (ret) { ++ if (err) { + rtw_warn(rtwdev, -+ "Failed to read %zu byte(s) from SDIO port 0x%08x", -+ bytes, rxaddr); -+ break; ++ "Failed to read %zu byte(s) from SDIO port 0x%08x: %d", ++ bytes, rxaddr, err); ++ ++ /* Signal to the caller that reading did not work and ++ * that the data in the buffer is short/corrupted. ++ */ ++ ret = err; ++ ++ /* Don't stop here - instead drain the remaining data ++ * from the card's buffer, else the card will return ++ * corrupt data for the next rtw_sdio_read_port() call. ++ */ + } + + count -= bytes; @@ -86,7 +125,7 @@ index 2c1fb2dabd40..b19262ec5d8c 100644 if (bus_claim) sdio_release_host(rtwsdio->sdio_func); -- -2.41.0 +2.42.1 From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Ping-Ke Shih diff --git a/patch/misc/rtw88/6.7/001-drivers-net-wireless-realtek-rtw88-upstream-wireless.patch b/patch/misc/rtw88/6.7/001-drivers-net-wireless-realtek-rtw88-upstream-wireless.patch index c7b7fce38..761cd4d37 100644 --- a/patch/misc/rtw88/6.7/001-drivers-net-wireless-realtek-rtw88-upstream-wireless.patch +++ b/patch/misc/rtw88/6.7/001-drivers-net-wireless-realtek-rtw88-upstream-wireless.patch @@ -1,8 +1,7 @@ From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Martin Blumenstingl -To: linux-wireless@vger.kernel.org -Date: Wed, 2 Aug 2023 00:27:32 +0000 -Subject: [PATCH] wifi: rtw88: sdio: Honor the host max_req_size in the RX path +Date: Mon, 20 Nov 2023 12:57:26 +0100 +Subject: [PATCH v3] wifi: rtw88: sdio: Honor the host max_req_size in the RX path Lukas reports skb_over_panic errors on his Banana Pi BPI-CM4 which comes with an Amlogic A311D (G12B) SoC and a RTL8822CS SDIO wifi/Bluetooth @@ -10,9 +9,10 @@ combo card. The error he observed is identical to what has been fixed in commit e967229ead0e ("wifi: rtw88: sdio: Check the HISR RX_REQUEST bit in rtw_sdio_rx_isr()") but that commit didn't fix Lukas' problem. -Lukas found that disabling or limiting RX aggregation fix the problem -for him. In the following discussion a few key topics have been -discussed which have an impact on this problem: +Lukas found that disabling or limiting RX aggregation works around the +problem for some time (but does not fully fix it). In the following +discussion a few key topics have been discussed which have an impact on +this problem: - The Amlogic A311D (G12B) SoC has a hardware bug in the SDIO controller which prevents DMA transfers. Instead all transfers need to go through the controller SRAM which limits transfers to 1536 bytes @@ -25,7 +25,7 @@ discussed which have an impact on this problem: be aggregated, BIT_DMA_AGG_TO_V1 configures a timeout for aggregation and BIT_EN_PRE_CALC makes the chip honor the limits more effectively) -Use multiple consecutive reads in rtw_sdio_read_port() to limit the +Use multiple consecutive reads in rtw_sdio_read_port() and limit the number of bytes which are copied by the host from the card in one MMC/SDIO transfer. This allows receiving a buffer that's larger than the hosts max_req_size (number of bytes which can be transferred in @@ -34,28 +34,58 @@ is gone as the rtw88 driver is now able to receive more than 1536 bytes from the card (either because the incoming packet is larger than that or because multiple packets have been aggregated). +In case of an receive errors (-EILSEQ has been observed by Lukas) we +need to drain the remaining data from the card's buffer, otherwise the +card will return corrupt data for the next rtw_sdio_read_port() call. + Fixes: 65371a3f14e7 ("wifi: rtw88: sdio: Add HCI implementation for SDIO based chipsets") Reported-by: Lukas F. Hartmann Closes: https://lore.kernel.org/linux-wireless/CAFBinCBaXtebixKbjkWKW_WXc5k=NdGNaGUjVE8NCPNxOhsb2g@mail.gmail.com/ Suggested-by: Ping-Ke Shih Signed-off-by: Martin Blumenstingl --- - drivers/net/wireless/realtek/rtw88/sdio.c | 24 +++++++++++++++++------ - 1 file changed, 18 insertions(+), 6 deletions(-) + +Changes since v2 at [2]: +- Don't initialize err to zero as that intiial value is never used. + Thanks Ping-Ke for spotting this! +- Add a comment explaning why we need to continue reading but still + have to return an error to the caller of rtw_sdio_read_port() + +Changes since v1 at [0]: +- We need to read all bytes if we split the transaction into multiple + smaller reads. This is even the case when one of N reads reports an + error. Otherwise the next read port call will return garbage (partially + containing zeros, ...). A similar-ish approach can be found in the + vendor driver, see [1] (specifically the call to sdio_recv_and_drop()) +- Update the patch description accordingly + +With a preliminary version of this updated patch Lukas reported off- +list: "i've been using this laptop for almost 3 hours with heavy wifi +usage and so far no problems" + + +[0] https://lore.kernel.org/lkml/169089906853.212423.17095176293160428610.kvalo@kernel.org/T/ +[1] https://github.com/chewitt/RTL8822CS/blob/ad1391e219b59314485739a499fb442d5bbc069e/hal/rtl8822c/sdio/rtl8822cs_io.c#L468-L477 +[2] https://lore.kernel.org/linux-wireless/20230806181656.2072792-1-martin.blumenstingl@googlemail.com/ + + + drivers/net/wireless/realtek/rtw88/sdio.c | 35 ++++++++++++++++++----- + 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/drivers/net/wireless/realtek/rtw88/sdio.c b/drivers/net/wireless/realtek/rtw88/sdio.c -index 2c1fb2dabd40..b19262ec5d8c 100644 +index 2c1fb2dabd40..0cae5746f540 100644 --- a/drivers/net/wireless/realtek/rtw88/sdio.c +++ b/drivers/net/wireless/realtek/rtw88/sdio.c -@@ -500,19 +500,31 @@ static u32 rtw_sdio_get_tx_addr(struct rtw_dev *rtwdev, size_t size, +@@ -500,19 +500,40 @@ static u32 rtw_sdio_get_tx_addr(struct rtw_dev *rtwdev, size_t size, static int rtw_sdio_read_port(struct rtw_dev *rtwdev, u8 *buf, size_t count) { struct rtw_sdio *rtwsdio = (struct rtw_sdio *)rtwdev->priv; + struct mmc_host *host = rtwsdio->sdio_func->card->host; bool bus_claim = rtw_sdio_bus_claim_needed(rtwsdio); u32 rxaddr = rtwsdio->rx_addr++; +- int ret; ++ int ret = 0, err; + size_t bytes; - int ret; if (bus_claim) sdio_claim_host(rtwsdio->sdio_func); @@ -69,14 +99,23 @@ index 2c1fb2dabd40..b19262ec5d8c 100644 + while (count > 0) { + bytes = min_t(size_t, host->max_req_size, count); + -+ ret = sdio_memcpy_fromio(rtwsdio->sdio_func, buf, ++ err = sdio_memcpy_fromio(rtwsdio->sdio_func, buf, + RTW_SDIO_ADDR_RX_RX0FF_GEN(rxaddr), + bytes); -+ if (ret) { ++ if (err) { + rtw_warn(rtwdev, -+ "Failed to read %zu byte(s) from SDIO port 0x%08x", -+ bytes, rxaddr); -+ break; ++ "Failed to read %zu byte(s) from SDIO port 0x%08x: %d", ++ bytes, rxaddr, err); ++ ++ /* Signal to the caller that reading did not work and ++ * that the data in the buffer is short/corrupted. ++ */ ++ ret = err; ++ ++ /* Don't stop here - instead drain the remaining data ++ * from the card's buffer, else the card will return ++ * corrupt data for the next rtw_sdio_read_port() call. ++ */ + } + + count -= bytes; @@ -86,5 +125,5 @@ index 2c1fb2dabd40..b19262ec5d8c 100644 if (bus_claim) sdio_release_host(rtwsdio->sdio_func); -- -2.41.0 +2.42.1