Received-SPF: pass (google.com: domain of linux-kernel+bounces-201470-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <b2cb86f7-c16a-44d2-a7b9-eb379784ff83@intel.com>
Date: Tue, 4 Jun 2024 15:25:41 -0700
User-Agent: Mozilla Thunderbird
Subject: Re: [net v3] net: ethernet: mtk_eth_soc: handle dma buffer size soc
 specific
To: Frank Wunderlich <linux@fw-web.de>, Felix Fietkau <nbd@nbd.name>, "Sean
 Wang" <sean.wang@mediatek.com>, Mark Lee <Mark-MC.Lee@mediatek.com>, "Lorenzo
 Bianconi" <lorenzo@kernel.org>, "David S. Miller" <davem@davemloft.net>,
	"Eric Dumazet" <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>, Paolo
 Abeni <pabeni@redhat.com>, Matthias Brugger <matthias.bgg@gmail.com>,
	AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
CC: Frank Wunderlich <frank-w@public-files.de>, John Crispin
	<john@phrozen.org>, <netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>, <linux-mediatek@lists.infradead.org>,
	<bc-bocun.chen@mediatek.com>, Daniel Golle <daniel@makrotopia.org>
References: <20240603192505.217881-1-linux@fw-web.de>
Content-Language: en-US
From: Jacob Keller <jacob.e.keller@intel.com>
In-Reply-To: <20240603192505.217881-1-linux@fw-web.de>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?N2JveWF3RE5rdGZLS2w0U0ZhekgxSTIvZ3MveUlhbVJIcHFsam50eGVwOWtE?=
 =?utf-8?B?ZFpkckNMeThVcFdCNEplbnpvVUo2ZVMzRGNoSElNSGhQamNkTHpkakkxL0sx?=
 =?utf-8?B?N1hRZkVHVmIwMk1CaHhHVkc2N3RUaEVINk9ZWmQySmFpUVpHVkxsUTZUZk8z?=
 =?utf-8?B?aWRqMTFicGV2QUIzRnhPS2VLQ0lpS05pRnlkL21zNUlTRmcvSktmTVEvajFR?=
 =?utf-8?B?aGxIck9OQWIwdWh2Uzg4Y1B4OFpOUWcvT0x0UXluWVQ3Y0l1dEdKczJJZHV4?=
 =?utf-8?B?M2Z1REkyM0JGL25oUUF2bU13VVVuajJveXpiazdZa0VhN0R4akYydU9pdUVj?=
 =?utf-8?B?dysvMU9xcld0QmxLQkdSSnQwTldVMXljN2ZTdzhhVXBUR2dVRkVMOHE0R213?=
 =?utf-8?B?cjZNSGhjUGJvR2g5Z3dnbUhRVW1DMytrMEppbkliNGsyL0N0Q3JRV1ZIZ3Bx?=
 =?utf-8?B?V1hpeGZRMUxjTERVaXVvUElwR2FkSVR4SFdURWZqNTg0cWJoL29Pc0QrTjZq?=
 =?utf-8?B?MGJTRUFaTUNpUXR5dDNiWGNobkFUb1NNaENNdjhxREZhZ1AyRXNXUjUzbXhm?=
 =?utf-8?B?bjVtaGFMcVh4T2E1aDgxMWFyL01ZOU1lcXpneFFEMk5na1hjM2xHSFZvMW1S?=
 =?utf-8?B?bi9taHZVM1Y2eUx1cDhseS9zK0lOWUhtMjIzakRrWG1LVUNvYUpQdXpOSGFM?=
 =?utf-8?B?bitUaUY3a2xWRmtLQmZ5Y2x4aGVQUTZVSmhVNHVWVExKaFlzUnlsS0hjcnNU?=
 =?utf-8?B?N1lXZVkwZDB2OFpvVzBjWjdrUjlhQWlEbWF5SjZPUEI0Ui91ZCtvUkVtOXBE?=
 =?utf-8?B?WTNHcnc4ZTcxcXp1MjdHVURJK1llcU1qOGdMSzhvQVlDN0pHVmw3TTRSSWZi?=
 =?utf-8?B?dXlSK3pma3JDaGVzSDl3VGJZRTZpYTZEbzlYeVlDdlN2QUUyelc4cHdPZWRw?=
 =?utf-8?B?V05ERTVqeWZCeEVtZCtxdTBwVHkrZmorTGIvN1lwd3EwMk9GK1hOK1FIb0tv?=
 =?utf-8?B?ZDZCTmVOU1pMUjJ2V2V3S2owaDA1VFBCbTlBemNkak4wL011NXAwUGpmYkhq?=
 =?utf-8?B?T2xRQXZuWE1ET2p0MTJzQU9DaHRISVM2OVBWbmtGUUNGVFJRa3NxL2JaV1Jt?=
 =?utf-8?B?azlwODVWb3NNN2xTUFZ6UUN5Y2FrWHNTNmZzNG9tRmlmanBrbUIrSnBRQThO?=
 =?utf-8?B?emNSVmkxalVwR21HZDJoenZVYkNCdEpGMi9PcFpyZHpJZVUwTkxZa2hIS2dX?=
 =?utf-8?B?My9Ydkp6cGxIZ1AycXNxUWNtVUgzMjgzc2IzNmxlcCtFNDkwS1ZZNEtWYTFk?=
 =?utf-8?B?M09Pdm41bzlNUTM3enFDWHRaREFtUmRYaVlhN0RpdXloYTF5RDVhNDBHZmd2?=
 =?utf-8?B?R2ZtbFV1ZXNCWEdyVWtvUllNajRoMjQrSW5PK3N6LzM2SXBubEo5L2tCV2pl?=
 =?utf-8?B?UGd4bHNQaDFGbE5aZnY4R2ZBU1lWU05JN1hzNVJ3WlFrR0lSSWJsd3pMckx3?=
 =?utf-8?B?RnpYUEFuVDVSMmVFNEg2b1hVanRsNWk3MGZQK2QzbTJXaW9FU3NPUEpMQ2ht?=
 =?utf-8?B?U0pTN1dJOGI1N1NwSkRaZDN2MlZNVlBrSld4dGZCLy9SWDhxUWFEU0ptZXpv?=
 =?utf-8?B?U0R0THhtbElSdldaNmFHY2xRVENMN1hYUVFoR0xsL0s5Mzl4ZXhRQnJPVk1P?=
 =?utf-8?B?bEtQOE85bi9mcjlSaFkwdWFnU3MrY29hUyt5NFUwOWJoaExpcUdHclI0MFNh?=
 =?utf-8?B?eHc0eU9ZUzdXdkZQZ28zSm9tSmo5bUpjZkNHTHh2OEhOd3ZRZERQMzNTVk94?=
 =?utf-8?B?Q21hNEFFbXllNmdORmcxYTF3eDU3ZWpTbCtZZEdkQm1XbkxoTThYMjVLbkxK?=
 =?utf-8?B?cTdYblVHeXAxY3JRN2ZtTHRhR3JGcjlFbFVTU0ZUbTJ5WHNXWVlDcW5BaGdF?=
 =?utf-8?B?RlEzdktVMzZHc2o1Z2tTbTNOQUVaaU00OGt4WnFVS0F1MnNsRGc1UGxTUGlZ?=
 =?utf-8?B?UVkzTGM2UnNlM1VWNTBuNFZaQnVINEtEdW5PUkpJZjVPcC9tL1FZb3oycEFp?=
 =?utf-8?B?S2hJTkdla01ZbE96MGxKN2FLdzhpU09Fc0V5RUQ0eG5qb0tEeDl1NkRZTTNK?=
 =?utf-8?B?bEcydHQ4WnY4bE8raUtuTm1DYjFBUVNPTEFZK1l3azZUb0lSbFhqTXNib0RZ?=
 =?utf-8?B?M3c9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 74683846-7e58-4b40-ac8d-08dc84e54621
X-MS-Exchange-CrossTenant-AuthSource: CO1PR11MB5089.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Jun 2024 22:25:43.9518
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: Uc/ZGsOZJSEPNEv+3pS0DvuxBhTh8IwjCfb/mNNjq9tq8ba7tmyHiTKY5b9tmqGUWtKgmwp7cgw0i86FZ4zqNjDrYyZcU4GzvoUTTyv/N3I=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO1PR11MB5204
X-OriginatorOrg: intel.com


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@intel.com>