Re: [PATCH net v2 1/2] net: ethernet: mediatek: split tx and rx fields in mtk_soc_data struct

[Przemek Kitszel]

On 5/7/24 14:24, Daniel Golle wrote:
> From: Lorenzo Bianconi <lorenzo@xxxxxxxxxx>
>
> 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@xxxxxxxxxx>
> Signed-off-by: Daniel Golle <daniel@xxxxxxxxxxxxxx>
> ---
> 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@xxxxxxxxx>

(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)