Re: [net v3] net: ethernet: mtk_eth_soc: handle dma buffer size soc specific

[Jacob Keller]

On 6/3/2024 12:25 PM, Frank Wunderlich wrote:
> @@ -1142,40 +1142,46 @@ static int mtk_init_fq_dma(struct mtk_eth *eth)
>  						       cnt * soc->tx.desc_size,
>  						       &eth->phy_scratch_ring,
>  						       GFP_KERNEL);
> +
>  	if (unlikely(!eth->scratch_ring))
>  		return -ENOMEM;
>  
> -	eth->scratch_head = kcalloc(cnt, MTK_QDMA_PAGE_SIZE, GFP_KERNEL);
> -	if (unlikely(!eth->scratch_head))
> -		return -ENOMEM;
> +	phy_ring_tail = eth->phy_scratch_ring + soc->tx.desc_size * (cnt - 1);
>  
> -	dma_addr = dma_map_single(eth->dma_dev,
> -				  eth->scratch_head, cnt * MTK_QDMA_PAGE_SIZE,
> -				  DMA_FROM_DEVICE);
> -	if (unlikely(dma_mapping_error(eth->dma_dev, dma_addr)))
> -		return -ENOMEM;
> +	for (j = 0; j < DIV_ROUND_UP(soc->tx.fq_dma_size, MTK_FQ_DMA_LENGTH); j++) {
> +		len = min_t(int, cnt - j * MTK_FQ_DMA_LENGTH, MTK_FQ_DMA_LENGTH);
> +		eth->scratch_head[j] = kcalloc(len, MTK_QDMA_PAGE_SIZE, GFP_KERNEL);
>  
> -	phy_ring_tail = eth->phy_scratch_ring + soc->tx.desc_size * (cnt - 1);
> +		if (unlikely(!eth->scratch_head[j]))
> +			return -ENOMEM;
>  
> -	for (i = 0; i < cnt; i++) {
> -		dma_addr_t addr = dma_addr + i * MTK_QDMA_PAGE_SIZE;
> -		struct mtk_tx_dma_v2 *txd;
> +		dma_addr = dma_map_single(eth->dma_dev,
> +					  eth->scratch_head[j], len * MTK_QDMA_PAGE_SIZE,
> +					  DMA_FROM_DEVICE);
>  
> -		txd = eth->scratch_ring + i * soc->tx.desc_size;
> -		txd->txd1 = addr;
> -		if (i < cnt - 1)
> -			txd->txd2 = eth->phy_scratch_ring +
> -				    (i + 1) * soc->tx.desc_size;
> +		if (unlikely(dma_mapping_error(eth->dma_dev, dma_addr)))
> +			return -ENOMEM;
>  
> -		txd->txd3 = TX_DMA_PLEN0(MTK_QDMA_PAGE_SIZE);
> -		if (MTK_HAS_CAPS(soc->caps, MTK_36BIT_DMA))
> -			txd->txd3 |= TX_DMA_PREP_ADDR64(addr);
> -		txd->txd4 = 0;
> -		if (mtk_is_netsys_v2_or_greater(eth)) {
> -			txd->txd5 = 0;
> -			txd->txd6 = 0;
> -			txd->txd7 = 0;
> -			txd->txd8 = 0;
> +		for (i = 0; i < cnt; i++) {
> +			struct mtk_tx_dma_v2 *txd;
> +
> +			txd = eth->scratch_ring + (j * MTK_FQ_DMA_LENGTH + i) * soc->tx.desc_size;
> +			txd->txd1 = dma_addr + i * MTK_QDMA_PAGE_SIZE;
> +			if (j * MTK_FQ_DMA_LENGTH + i < cnt)
> +				txd->txd2 = eth->phy_scratch_ring +
> +					    (j * MTK_FQ_DMA_LENGTH + i + 1) * soc->tx.desc_size;
> +
> +			txd->txd3 = TX_DMA_PLEN0(MTK_QDMA_PAGE_SIZE);
> +			if (MTK_HAS_CAPS(soc->caps, MTK_36BIT_DMA))
> +				txd->txd3 |= TX_DMA_PREP_ADDR64(dma_addr + i * MTK_QDMA_PAGE_SIZE);
> +
> +			txd->txd4 = 0;
> +			if (mtk_is_netsys_v2_or_greater(eth)) {
> +				txd->txd5 = 0;
> +				txd->txd6 = 0;
> +				txd->txd7 = 0;
> +				txd->txd8 = 0;
> +			}

This block of change was a bit hard to understand what was going on, but
I think I get the result is that you end up allocating different set of
scratch_head per size vs the original only having one scratch_head per
device?

Perhaps you can explain, but we're now allocating a bunch of different
scratch_head pointers.. However, in the patch, the only places that we
modify scratch_head appear to be the allocation path and the free path..
but I can't seem to understand how that would impact the users of
scratch head? I guess it changes the dma_addr which then changes the txd
values we program?

Ok.

I sort of understand whats going on here, but it was a fair bit to fully
grok this flow.

Overall, I'm no expert on the part or DMA here, but:

Reviewed-by: Jacob Keller <jacob.e.keller@xxxxxxxxx>