Received-SPF: pass (google.com: domain of linux-kernel+bounces-171720-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Message-ID: <2a5459a7-954c-4841-8ad5-ac93596fc21f@intel.com>
Date: Tue, 7 May 2024 17:47:08 +0200
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH net v2 1/2] net: ethernet: mediatek: split tx and rx
 fields in mtk_soc_data struct
Content-Language: en-US
To: Daniel Golle <daniel@makrotopia.org>, 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>,
	<netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>, <linux-mediatek@lists.infradead.org>
References: <6747c038f4fc3b490e1b7a355cdaaf361e359def.1715084578.git.daniel@makrotopia.org>
From: Przemek Kitszel <przemyslaw.kitszel@intel.com>
In-Reply-To: <6747c038f4fc3b490e1b7a355cdaaf361e359def.1715084578.git.daniel@makrotopia.org>
Content-Type: text/plain; charset="UTF-8"; format=flowed
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?SnFuOCt3d2RWTDBiUS9xdzhlL0hZZjFuT3dHNVE1RXdVOGF5TmRYUjd0c1hI?=
 =?utf-8?B?OVdxbm1DTEtCV0gxM2NJd2xQN1VxVTR1OWhOb0RzVGFKdW5HVVU3ZFpNVXhq?=
 =?utf-8?B?Qk02UkNITFBhSCtlMklnVHd6b0xYWWJjZ1Z2SGUvTHZFU1ovblZjeGpUZERy?=
 =?utf-8?B?WUd5ZWVSVnBHL2thS244MjNHQUFaS3llZVk5RkRJNkgvVnNuSWxVd2F0a05T?=
 =?utf-8?B?RTVaQVhEb2x1OGJaQVNkZ2piaGxiQzg2Wk5zeExESFcxcGh0R2lJV09MNGk3?=
 =?utf-8?B?RGs5OXJwZ1pvZlZDdWNYNzZuMmNhd3FCQnp3dzNwVFFyREdlL3MxVUIzZm9B?=
 =?utf-8?B?d2FhaHRDcnQvRG5PZFMxSjhKcWJWcytqWjFiSCswWXpreUZmK1hDYnJFQlZR?=
 =?utf-8?B?ZFNHYkp2QTNNaXEzOExNTWdnZkV1U3lJZTJvOFgwckxaa1dxbmcvN2s2bUQ1?=
 =?utf-8?B?L2JqSkpYZ3c3b1B2SFN5RTllRTRYNkhKOERMWmp1bTVjSnIxTGJ4Mlljcmph?=
 =?utf-8?B?MVBrVnp5K3R2NFM5MTdua1hCeG1lN2lndWNKR1dNTit2a2Z0K3dkc1VYVmU5?=
 =?utf-8?B?aDd5czRVRWsyYUNkbG5VY0ZaYk5VREtiTWVuckdKN1p3WlI3bW1yVlhGV0RT?=
 =?utf-8?B?b3c3OVBpdWt1bDFVbzNBM0dvSW9tOW9UMlFzZHd6SWNHQTNLVUtXVGRiRjBs?=
 =?utf-8?B?OGhnazYzUktCeWNCaC9yYWN5UUU2eGFnVGVtendzRTFMTTQ4bGRpZjNVSXpB?=
 =?utf-8?B?R1RlWmszTTNvelJpaUpHL3NiZjhGODlaL29wcGoyMUFkNXl0MHROazNvYUxL?=
 =?utf-8?B?dXNEa3lVR1I5bGpHV3BGZ2hVNmlTZCtyODZaRVhoRkV1WmMxV3J6MU5uSlha?=
 =?utf-8?B?QlJ6eHpYdE1oZXhTNlFyNzRuN2NWdndsUEdFTlMxckpHbGFNMlZVMGt1aHcz?=
 =?utf-8?B?bmsvQjk3bEJ5d2twcDB3QW9udGhBYzFrNFVORnNYUVppTjFkOTJPNUlZVXNr?=
 =?utf-8?B?NE5kZkJEYTBTRkZxTDdCUFdxVGp3SEZ6TWdINlVaaWNRdktwdTEyVm5KaEFq?=
 =?utf-8?B?RTB1V2FWV1B2MVdxSHNneVZrOWpmaEpqOFZ6UnRvTWVQMmJlK2hRTS9rc0M2?=
 =?utf-8?B?T1JEL0pQN3V3eFIrK1VEdy9QK1BNRGx3RmdzWUVtdlBjYnVrSGFGOWxhKys2?=
 =?utf-8?B?bHlQSnFkMVoxVmptV1hWaVVPZE9ad05KbEJQSFBjQk1qUkkrUTgzRjM3alJ6?=
 =?utf-8?B?QjE1VjExSXZXWFVuMzNIU3A2WlhpT0xoSkZXMGw5YWVOMW5PTTMvb0Vua1Jr?=
 =?utf-8?B?RlZrOHhYL013ZTlJQ3dsLzhUclBiTmFaQ0s5SUR0Mm9yemRCNVBFdFdDU1Bu?=
 =?utf-8?B?cEpBNEFZNWNHR1dxSHNPTStnZFdGNnovbE50aWRyOURCS2VsMS9KRnJLdkpT?=
 =?utf-8?B?MEZPZVVjaURPSDI5Zmx0WVNFNUNITVRLWVhuclIra3o2UGpHYjlvSkl1V3hY?=
 =?utf-8?B?bmFYOW1VQkpIbGZXNVhQTEZPSmc4U1ZpUTg1cWJSSTRkeThYcm1XaFJrdTRT?=
 =?utf-8?B?OHQ4aDQ0Q3dydGJvY2ZEWFd6cS9lNWNnY25hQm1zL3dUeGt2SFdnUmR4VWJC?=
 =?utf-8?B?S3BOeGh1V3hjTzJKWHZWMHBBMFJSV3I0Z1FQVlNzaDJFQ1YyUVMzYXlMTm9v?=
 =?utf-8?B?cVpGTTZYc1laSmpickZEc0V2ZXFHNFJGV1VtYnV6eXJ4ZXJGeGFhQmtZQTZV?=
 =?utf-8?B?ZXlxem9Ra1pvU0ZIUzBtR0x1c2c0NFViY1VoY2M4ZDlNM1JZeHZMMXFMK0hK?=
 =?utf-8?B?ZjZTQlloaGVVSWxWd2pRTzdHVExFSDMyaENiaDV3dGVBVHJRVEVFUzc0Um1p?=
 =?utf-8?B?OTRadEVIZ2VmR0xFWlN5aWw5bGluTU5YKzlZODBZYTFseUpLaWZyeDYycGR0?=
 =?utf-8?B?eGZJWDJGZ3pkRmlGTGdzZm80c0VRd002ODN6dFBxTkw1NHlGeU5NenF4czh4?=
 =?utf-8?B?OVZYR3I1SUlhaVdwd2pRVXpJYXNyeVljNU5jcThwMTI2M1B3SUlOeU5lZWJ2?=
 =?utf-8?B?MW5FcGNkYm1qQmozcmJnMktpSXV6YUhoSXU1V2NkQTlWNVhiTGRqVTR2T1B1?=
 =?utf-8?B?YTQzeFpyNnBHbHhXaSt0eHNTaTJXNmVCcERYUmJNTzYwa1BiT3l0YzVnb2dD?=
 =?utf-8?B?bUE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 780f638d-37db-4125-7b31-08dc6eacfa58
X-MS-Exchange-CrossTenant-AuthSource: MN6PR11MB8102.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 07 May 2024 15:47:19.4634
 (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: 5loo1Ju8m2gm74FtdS+8FrKBGuiGqCGoQG2AACfau+sj5VHcYFKxSBGK9KNay+Rmj8XAB5b4LN6eQhLaKWmAyM39rIL0ZXAW00BJj6HTFnQ=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR11MB5207
X-OriginatorOrg: intel.com

On 5/7/24 14:24, Daniel Golle wrote:
> From: Lorenzo Bianconi <lorenzo@kernel.org>
> 
> Split tx and rx fields in mtk_soc_data struct. This is a preliminary
> patch to roll back to ADMAv1 for MT7986 and MT7981 SoC in order to fix a
> hw hang if the device receives a corrupted packet when using ADMAv2.0.
> 
> Fixes: 197c9e9b17b1 ("net: ethernet: mtk_eth_soc: introduce support for mt7986 chipset")
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> ---
> v2: improve commit message
> 
>   drivers/net/ethernet/mediatek/mtk_eth_soc.c | 210 ++++++++++++--------
>   drivers/net/ethernet/mediatek/mtk_eth_soc.h |  29 +--
>   2 files changed, 139 insertions(+), 100 deletions(-)
> 

Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>

(please find some possible followups below)

// ...

> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> index 9ae3b8a71d0e..39b50de1decb 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h

// ...

> @@ -1153,10 +1153,9 @@ struct mtk_reg_map {
>    * @foe_entry_size		Foe table entry size.
>    * @has_accounting		Bool indicating support for accounting of
>    *				offloaded flows.
> - * @txd_size			Tx DMA descriptor size.
> - * @rxd_size			Rx DMA descriptor size.
> - * @rx_irq_done_mask		Rx irq done register mask.
> - * @rx_dma_l4_valid		Rx DMA valid register mask.
> + * @desc_size			Tx/Rx DMA descriptor size.

I find it a bit misleading that you could document fields of named
members at top level like that, but this is not an issue to be resolved
via -net patch.

> + * @irq_done_mask		Rx irq done register mask.
> + * @dma_l4_valid		Rx DMA valid register mask.
>    * @dma_max_len			Max DMA tx/rx buffer length.
>    * @dma_len_offset		Tx/Rx DMA length field offset.
>    */
> @@ -1174,13 +1173,17 @@ struct mtk_soc_data {
>   	bool		has_accounting;
>   	bool		disable_pll_modes;
>   	struct {
> -		u32	txd_size;
> -		u32	rxd_size;
> -		u32	rx_irq_done_mask;
> -		u32	rx_dma_l4_valid;
> +		u32	desc_size;
>   		u32	dma_max_len;
>   		u32	dma_len_offset;
> -	} txrx;
> +	} tx;
> +	struct {
> +		u32	desc_size;
> +		u32	irq_done_mask;
> +		u32	dma_l4_valid;
> +		u32	dma_max_len;
> +		u32	dma_len_offset;
> +	} rx;

you could consired a followup that reorders fields to fillup holes

>   };
>   
>   #define MTK_DMA_MONITOR_TIMEOUT		msecs_to_jiffies(1000)