Re: [PATCH] net: airoha: npu: use coherent DMA for mailbox messages

> Commit 6f884eb87a79 ("net: airoha: Fix DMA direction for NPU mailbox
> buffer") switched airoha_npu_send_msg() to DMA_BIDIRECTIONAL so
> non-coherent CPUs invalidate caches before reading NPU GET responses.
> 
> On EN7581 + MT7996 that change regresses probe: the mailbox completes
> successfully, but WLAN_FUNC_GET_WAIT_NPU_VERSION still reads as 0.0 and
> mt76 never binds NPU offload. Healthy boards report 0.1111.
> 
> Mailbox messages are tiny and already copied by the caller. Bounce
> through dma_alloc_coherent() so the CPU observes the NPU-written
> response without relying on streaming DMA direction.
> 
> Verified on Quantum Fiber W1700K (EN7581 + MT7996).
> 
> Fixes: 6f884eb87a79 ("net: airoha: Fix DMA direction for NPU mailbox buffer")
> Assisted-by: Cursor:composer-2
> Signed-off-by: Daniel Pawlik <[email protected]>
> ---
>  drivers/net/ethernet/airoha/airoha_npu.c | 21 ++++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/ethernet/airoha/airoha_npu.c b/drivers/net/ethernet/airoha/airoha_npu.c
> index b679bed952de..d2f6c9c084b8 100644
> --- a/drivers/net/ethernet/airoha/airoha_npu.c
> +++ b/drivers/net/ethernet/airoha/airoha_npu.c
> @@ -166,12 +166,21 @@ static int airoha_npu_send_msg(struct airoha_npu *npu, int func_id,
>  	u16 core = 0; /* FIXME */
>  	u32 val, offset = core << 4;
>  	dma_addr_t dma_addr;
> +	void *dma_buf;
>  	int ret;
>  
> -	dma_addr = dma_map_single(npu->dev, p, size, DMA_BIDIRECTIONAL);
> -	ret = dma_mapping_error(npu->dev, dma_addr);
> -	if (ret)
> -		return ret;
> +	/*
> +	 * Mailbox payloads are small and bidirectional (CPU sets the
> +	 * request, NPU writes the response). Streaming DMA_BIDIRECTIONAL
> +	 * mapping regresses EN7581+MT7996: MBOX reports success but
> +	 * WLAN_FUNC_GET_WAIT_NPU_VERSION still reads as 0.0. Use a
> +	 * coherent bounce buffer so the CPU always sees the NPU response.
> +	 */
> +	dma_buf = dma_alloc_coherent(npu->dev, size, &dma_addr, GFP_ATOMIC);
> +	if (!dma_buf)
> +		return -ENOMEM;

I tried to reproduce the issue locally, but it does not occur for me.
Moreover, I guess dma_alloc_coherent() is not the right approach since it is
usually used for long-standing descriptors (e.g. tx/rx DMA descriptor rings).
Can you please provide more details about the hw you are running?

Regards,
Lorenzo

> +
> +	memcpy(dma_buf, p, size);
>  
>  	spin_lock_bh(&npu->cores[core].lock);
>  
> @@ -191,7 +200,9 @@ static int airoha_npu_send_msg(struct airoha_npu *npu, int func_id,
>  
>  	spin_unlock_bh(&npu->cores[core].lock);
>  
> -	dma_unmap_single(npu->dev, dma_addr, size, DMA_BIDIRECTIONAL);
> +	if (!ret)
> +		memcpy(p, dma_buf, size);
> +	dma_free_coherent(npu->dev, size, dma_buf, dma_addr);
>  
>  	return ret;
>  }
> -- 
> 2.55.0
>

-----BEGIN PGP SIGNATURE-----

iHUEABYKAB0WIQTquNwa3Txd3rGGn7Y6cBh0uS2trAUCanMJcgAKCRA6cBh0uS2t
rKWZAQCujgPJTZmcbTNA0/ICmZK2kc73Mi+wEdDGBnP49Ka4iQD+ONR1+zHNLbXx
445U0plbBScvVOcAFOFBID5pan51xws=
=gjZ7
-----END PGP SIGNATURE-----