Re: [PATCH 13/13] wifi: brcmfmac: Add BCM4388 support

[Janne Grunau <j@jannau.net>]

On Fri, Sep 25, 2026 at 11:43:48PM +1000, Michael Reeves via B4 Relay wrote:
> From: Hector Martin <marcan@marcan.st>
> 
> Add BCM4388 firmware mappings, RAM base and OTP handling, and increase
> the RX posting and completion ring capacities to 2048 items.
> 
> On BCM4388, DMA_FROM_DEVICE mappings for posted receive buffers cause
> DART read-permission faults followed by firmware DMA failure. Map these
> buffers with DMA_BIDIRECTIONAL to allow device reads as well as writes.

Not only on BCM4388. See commit 7ed5dce55fa8 ("wifi: brcmfmac: map msgbuf
RX packet IDs bidirectional") [1] only in wireless/main, doesn't seem to
made it into next yet. BCM4378 and BCM4387 do need the RX msgbufs mapped
as bidirectional as well.

Link: https://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless.git/commit/?id=7ed5dce55fa8393972b1bf2bf5af1c818c5e7dc2 [1]

> Keep the existing mapping direction for other chips.
> 
> Signed-off-by: Hector Martin <marcan@marcan.st>
> Co-developed-by: Michael Reeves <michael.reeves077@gmail.com>
> Signed-off-by: Michael Reeves <michael.reeves077@gmail.com>
> ---
>  drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c      |  1 +
>  drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c    |  8 +++++---
>  drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.h    |  4 ++--
>  drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c      | 12 +++++++++++-
>  .../net/wireless/broadcom/brcm80211/include/brcm_hw_ids.h    |  2 ++
>  5 files changed, 21 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
> index 253ab45c33..411188010f 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
> @@ -751,6 +751,7 @@ static u32 brcmf_chip_tcm_rambase(struct brcmf_chip_priv *ci)
>  	case BRCM_CC_4366_CHIP_ID:
>  	case BRCM_CC_43664_CHIP_ID:
>  	case BRCM_CC_43666_CHIP_ID:
> +	case BRCM_CC_4388_CHIP_ID:
>  		return 0x200000;
>  	case BRCM_CC_4355_CHIP_ID:
>  	case BRCM_CC_4359_CHIP_ID:
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c
> index 8e5c697e7f..5ad46d7ec6 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c
> @@ -12,6 +12,7 @@
>  #include <linux/netdevice.h>
>  #include <linux/etherdevice.h>
>  
> +#include <brcm_hw_ids.h>
>  #include <brcmu_utils.h>
>  #include <brcmu_wifi.h>
>  
> @@ -51,7 +52,7 @@
>  #define MSGBUF_TYPE_D2H_MAILBOX_DATA		0x24
>  
>  #define NR_TX_PKTIDS				2048
> -#define NR_RX_PKTIDS				1024
> +#define NR_RX_PKTIDS				2048
>  
>  #define BRCMF_IOCTL_REQ_PKTID			0xFFFE
>  
> @@ -330,7 +331,6 @@ brcmf_msgbuf_init_pktids(u32 nr_array_entries,
>  	return pktids;
>  }
>  
> -

nit: cosmetic change

>  static int
>  brcmf_msgbuf_alloc_pktid(struct device *dev,
>  			 struct brcmf_msgbuf_pktids *pktids,
> @@ -1695,8 +1695,10 @@ int brcmf_proto_msgbuf_attach(struct brcmf_pub *drvr)
>  						     DMA_TO_DEVICE);
>  	if (!msgbuf->tx_pktids)
>  		goto fail;
> +	/* BCM4388 firmware also reads from posted RX buffers. */
>  	msgbuf->rx_pktids = brcmf_msgbuf_init_pktids(NR_RX_PKTIDS,
> -						     DMA_FROM_DEVICE);
> +			drvr->bus_if->chip == BRCM_CC_4388_CHIP_ID ?
> +			DMA_BIDIRECTIONAL : DMA_FROM_DEVICE);

This conflicts with the linked change. Best to drop this hunk and rely
on the linked change.

[...]

> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
> @@ -2459,7 +2468,7 @@ static int brcmf_pcie_read_otp(struct brcmf_pciedev_info *devinfo)
>  
>  static bool brcmf_pcie_request_fw_signature(struct brcmf_pciedev_info *devinfo)
>  {
> -	return false;
> +	return devinfo->ci->chip == BRCM_CC_4388_CHIP_ID;
>  }

nit: this is hard to extend.

```
        switch (devinfo->ci->chip) {
	case BRCM_CC_4388_CHIP_ID:
	        return true;
	default:
	        return false;
	}
```
would be easier to extend and is in my opinion easier to read

Janne