[PATCH wireless] wifi: b43: map the RX buffers DMA_BIDIRECTIONAL

[PATCH wireless] wifi: b43: map the RX buffers DMA_BIDIRECTIONAL

[Rosen Penev]

b43 poisons each RX buffer before it hands it to the device, and drops
a buffer that still carries the poison, as the device sometimes skips
one. The RX buffers are mapped and synced DMA_FROM_DEVICE. On a
non-coherent CPU like the BCM4716 or the BCM4708, the map and the sync
for the device then only invalidate the cache, so the poison written
just before never reaches memory. A skipped buffer is read as whatever
the memory held before, for a recycled buffer the data of its previous
use: garbage frames, or a length that sends dma_rx() into its path for
frames spanning several buffers.

Map, sync and unmap the RX buffers DMA_BIDIRECTIONAL, so that the
poison is written back before the device gets the buffer.

Reported on a Netgear R6400 (BCM4708, BCM4331).

Fixes: cf68636a9773 ("b43: Refresh RX poison on buffer recycling")
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/net/wireless/broadcom/b43/dma.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/broadcom/b43/dma.c b/drivers/net/wireless/broadcom/b43/dma.c
index 05da6987a845..c8585c6dff37 100644
--- a/drivers/net/wireless/broadcom/b43/dma.c
+++ b/drivers/net/wireless/broadcom/b43/dma.c
@@ -353,8 +353,14 @@ static inline
 		dmaaddr = dma_map_single(ring->dev->dev->dma_dev,
 					 buf, len, DMA_TO_DEVICE);
 	} else {
+		/*
+		 * Not DMA_FROM_DEVICE: on a non-coherent CPU, the map and
+		 * the sync for the device would then only invalidate the
+		 * cache, and the poison of b43_poison_rx_buffer() would
+		 * never reach memory.
+		 */
 		dmaaddr = dma_map_single(ring->dev->dev->dma_dev,
-					 buf, len, DMA_FROM_DEVICE);
+					 buf, len, DMA_BIDIRECTIONAL);
 	}
 
 	return dmaaddr;
@@ -369,7 +375,7 @@ static inline
 				 addr, len, DMA_TO_DEVICE);
 	} else {
 		dma_unmap_single(ring->dev->dev->dma_dev,
-				 addr, len, DMA_FROM_DEVICE);
+				 addr, len, DMA_BIDIRECTIONAL);
 	}
 }
 
@@ -379,7 +385,7 @@ static inline
 {
 	B43_WARN_ON(ring->tx);
 	dma_sync_single_for_cpu(ring->dev->dev->dma_dev,
-				    addr, len, DMA_FROM_DEVICE);
+				    addr, len, DMA_BIDIRECTIONAL);
 }
 
 static inline
@@ -388,7 +394,7 @@ static inline
 {
 	B43_WARN_ON(ring->tx);
 	dma_sync_single_for_device(ring->dev->dev->dma_dev,
-				   addr, len, DMA_FROM_DEVICE);
+				   addr, len, DMA_BIDIRECTIONAL);
 }
 
 static inline
-- 
2.56.0

Re: [PATCH wireless] wifi: b43: map the RX buffers DMA_BIDIRECTIONAL

[Michael Büsch]

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

Quite honestly, to me this looks like a lot of AI slop is sent to b43 these days.
Can you please slow down your submissions so that actual people get a chance to review the stuff?
Thanks!

-- 
Michael Büsch
https://bues.ch/


On Fri, 23 Oct 2026 12:06:53 -0700
Rosen Penev <rosenp@gmail.com> wrote:

> b43 poisons each RX buffer before it hands it to the device, and drops
> a buffer that still carries the poison, as the device sometimes skips
> one. The RX buffers are mapped and synced DMA_FROM_DEVICE. On a
> non-coherent CPU like the BCM4716 or the BCM4708, the map and the sync
> for the device then only invalidate the cache, so the poison written
> just before never reaches memory. A skipped buffer is read as whatever
> the memory held before, for a recycled buffer the data of its previous
> use: garbage frames, or a length that sends dma_rx() into its path for
> frames spanning several buffers.
> 
> Map, sync and unmap the RX buffers DMA_BIDIRECTIONAL, so that the
> poison is written back before the device gets the buffer.
> 
> Reported on a Netgear R6400 (BCM4708, BCM4331).
> 
> Fixes: cf68636a9773 ("b43: Refresh RX poison on buffer recycling")
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  drivers/net/wireless/broadcom/b43/dma.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/wireless/broadcom/b43/dma.c b/drivers/net/wireless/broadcom/b43/dma.c
> index 05da6987a845..c8585c6dff37 100644
> --- a/drivers/net/wireless/broadcom/b43/dma.c
> +++ b/drivers/net/wireless/broadcom/b43/dma.c
> @@ -353,8 +353,14 @@ static inline
>  		dmaaddr = dma_map_single(ring->dev->dev->dma_dev,
>  					 buf, len, DMA_TO_DEVICE);
>  	} else {
> +		/*
> +		 * Not DMA_FROM_DEVICE: on a non-coherent CPU, the map and
> +		 * the sync for the device would then only invalidate the
> +		 * cache, and the poison of b43_poison_rx_buffer() would
> +		 * never reach memory.
> +		 */
>  		dmaaddr = dma_map_single(ring->dev->dev->dma_dev,
> -					 buf, len, DMA_FROM_DEVICE);
> +					 buf, len, DMA_BIDIRECTIONAL);
>  	}
>  
>  	return dmaaddr;
> @@ -369,7 +375,7 @@ static inline
>  				 addr, len, DMA_TO_DEVICE);
>  	} else {
>  		dma_unmap_single(ring->dev->dev->dma_dev,
> -				 addr, len, DMA_FROM_DEVICE);
> +				 addr, len, DMA_BIDIRECTIONAL);
>  	}
>  }
>  
> @@ -379,7 +385,7 @@ static inline
>  {
>  	B43_WARN_ON(ring->tx);
>  	dma_sync_single_for_cpu(ring->dev->dev->dma_dev,
> -				    addr, len, DMA_FROM_DEVICE);
> +				    addr, len, DMA_BIDIRECTIONAL);
>  }
>  
>  static inline
> @@ -388,7 +394,7 @@ static inline
>  {
>  	B43_WARN_ON(ring->tx);
>  	dma_sync_single_for_device(ring->dev->dev->dma_dev,
> -				   addr, len, DMA_FROM_DEVICE);
> +				   addr, len, DMA_BIDIRECTIONAL);
>  }
>  
>  static inline



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH wireless] wifi: b43: map the RX buffers DMA_BIDIRECTIONAL

[Michael Büsch]

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

To explain this further:

- I might be wrong, but this patch looks completely bogus to me.
  Why would this be bi-directional?
  If this is accessed bi-directionally, there must be a bug somewhere else.
- Please provide a HUMAN WRITTEN patch description.
  This is really important.

I am all for using AI for development.
Especially for such old and legacy code.
HOWEVER, there must be a human present and understanding what is being done.

Please slow down your submissions.
Think about your patches before submission.
TEST your code before submission and send evidence of your tests.
Untested code is to be rejected.

-- 
Michael Büsch
https://bues.ch/


On Fri, 23 Oct 2026 22:14:24 +0200
Michael Büsch <m@bues.ch> wrote:

> Quite honestly, to me this looks like a lot of AI slop is sent to b43 these days.
> Can you please slow down your submissions so that actual people get a chance to review the stuff?
> Thanks!
> 

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH wireless] wifi: b43: map the RX buffers DMA_BIDIRECTIONAL

[Rosen Penev]

On Fri, Oct 23, 2026 at 1:42 PM Michael Büsch <m@bues.ch> wrote:
>
> To explain this further:
>
> - I might be wrong, but this patch looks completely bogus to me.
>   Why would this be bi-directional?
>   If this is accessed bi-directionally, there must be a bug somewhere else.
> - Please provide a HUMAN WRITTEN patch description.
>   This is really important.
>
> I am all for using AI for development.
> Especially for such old and legacy code.
> HOWEVER, there must be a human present and understanding what is being done.
>
> Please slow down your submissions.
> Think about your patches before submission.
> TEST your code before submission and send evidence of your tests.
All code is tested on an Asus RT-N16. It runs on non-coherent MIPS.
> Untested code is to be rejected.
>
> --
> Michael Büsch
> https://bues.ch/
>
>
> On Fri, 23 Oct 2026 22:14:24 +0200
> Michael Büsch <m@bues.ch> wrote:
>
> > Quite honestly, to me this looks like a lot of AI slop is sent to b43 these days.
> > Can you please slow down your submissions so that actual people get a chance to review the stuff?
> > Thanks!
> >

Re: [PATCH wireless] wifi: b43: map the RX buffers DMA_BIDIRECTIONAL

[Michael Büsch]

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

On Fri, 23 Oct 2026 13:44:44 -0700
Rosen Penev <rosenp@gmail.com> wrote:

> All code is tested on an Asus RT-N16. It runs on non-coherent MIPS.

Ok.

Describe your development process in human language during patch submission.
What has been tested?
What has not been tested?
What was wrong?
What has been fixed?
Why is this needed?
Why do we need bidirectional DMA and most important why wasn't it necessary before?

AI is great for code generation, bug finding, etc.
But for patch submission, pull requests and commit messages it's absolutely unusable.
Please don't do it.
Write your commit messages manually, please.

Please make your commit message be to the point and easy to review.
And please slow the process down.
You don't need to submit patches right away. Keep them in a queue and STABILIZE them LOCALLY.

Please do a little bit of research whom to To and Cc. There are scripts for that.

Thanks a lot.

-- 
Michael Büsch
https://bues.ch/

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH wireless] wifi: b43: map the RX buffers DMA_BIDIRECTIONAL

[Rosen Penev]

On Fri, Oct 23, 2026 at 1:58 PM Michael Büsch <m@bues.ch> wrote:
>
> On Fri, 23 Oct 2026 13:44:44 -0700
> Rosen Penev <rosenp@gmail.com> wrote:
>
> > All code is tested on an Asus RT-N16. It runs on non-coherent MIPS.
>
> Ok.
>
> Describe your development process in human language during patch submission.
I assume that would mostly be appropriate for a cover letter.
> What has been tested?
> What has not been tested?
> What was wrong?
> What has been fixed?
> Why is this needed?
> Why do we need bidirectional DMA and most important why wasn't it necessary before?
Because the codepath is not fatal. One or two duplicated frames. Big deal.
>
> AI is great for code generation, bug finding, etc.
> But for patch submission, pull requests and commit messages it's absolutely unusable.
> Please don't do it.
> Write your commit messages manually, please.
I've been burned by this many times before where maintainers ask me to
reword or mention something. It's not practical and wastes everyone's
time, which is why I'm almost banned from netdev.
>
> Please make your commit message be to the point and easy to review.
> And please slow the process down.
> You don't need to submit patches right away. Keep them in a queue and STABILIZE them LOCALLY.
I figure since wireless doesn't depend on the merge window unlike
wireless-next I can send them whenever.
>
> Please do a little bit of research whom to To and Cc. There are scripts for that.
Yeah. All of this goes through ./scripts/get_maintainer.pl

Do note this is an orphaned driver. I expect feature patches to get
clogged up and Fixes: to get in a bit easier.
>
> Thanks a lot.
>
> --
> Michael Büsch
> https://bues.ch/

Re: [PATCH wireless] wifi: b43: map the RX buffers DMA_BIDIRECTIONAL

[Michael Büsch]

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

On Fri, 23 Oct 2026 14:21:27 -0700
Rosen Penev <rosenp@gmail.com> wrote:

>> Why do we need bidirectional DMA and most important why wasn't it necessary before?  
>Because the codepath is not fatal. One or two duplicated frames. Big deal.

I do not understand what this means.
Please explain this in human language.
Why is the code wrong?
Why did it work for almost two decades?
Why does it need to change?
Please explain it to a human being with code examples and failure scenarios that are understandable.

It is completely Ok, if it *does* need to change.
I am just asking for an explanation that is not AI generated.

If you want me to read it, it should not be too much for you to type it down.

> > Write your commit messages manually, please.  
> I've been burned by this many times before where maintainers ask me to
> reword or mention something. It's not practical and wastes everyone's
> time, which is why I'm almost banned from netdev.

I'm sorry, you are wrong.
This is not how software development works.

If you can't explain what you submit, I am going to NAK everything.
(even if I don't have the formal power anymore, see below)

> Do note this is an orphaned driver.

Yes, I know. I am the former maintainer.

>I expect [..] Fixes: to get in a bit easier.

No.
Fixes need to be explained to avoid regressions.

-- 
Michael Büsch
https://bues.ch/

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]