[PATCH 5/8] net: dwc_eth_qos: add dwc eqos for imx support

[Daniil Stas]

Hi, i think there are some issues with this patch.

> @@ -1131,6 +1205,7 @@ static int eqos_start(struct udevice *dev)
>  	}
>  
>  	/* Configure MTL */
> +	writel(0x60, &eqos->mtl_regs->txq0_quantum_weight - 0x100);
>  
>  	/* Enable Store and Forward mode for TX */
>  	/* Program Tx operating mode */

What is this address: &eqos->mtl_regs->txq0_quantum_weight - 0x100?
Isn't it outside of MTL registers range?

> @@ -1144,7 +1219,9 @@ static int eqos_start(struct udevice *dev)
>  
>  	/* Enable Store and Forward mode for RX, since no jumbo frame */
>  	setbits_le32(&eqos->mtl_regs->rxq0_operation_mode,
> -		     EQOS_MTL_RXQ0_OPERATION_MODE_RSF);
> +		     EQOS_MTL_RXQ0_OPERATION_MODE_RSF |
> +		     EQOS_MTL_RXQ0_OPERATION_MODE_FEP |
> +		     EQOS_MTL_RXQ0_OPERATION_MODE_FUP);
>  
>  	/* Transmit/Receive queue fifo size; use all RAM for 1 queue */
>  	val = readl(&eqos->mac_regs->hw_feature1);

Why do you set FEP and FUP bits? It can lead to data corruption as they
allow accepting erroneous packets.

I think these options should only be used in some debugging mode but not
in production.

> @@ -1220,6 +1297,19 @@ static int eqos_start(struct udevice *dev)
>  			eqos->config->config_mac <<
>  			EQOS_MAC_RXQ_CTRL0_RXQ0EN_SHIFT);
>  
> +	clrsetbits_le32(&eqos->mac_regs->rxq_ctrl0,
> +			EQOS_MAC_RXQ_CTRL0_RXQ0EN_MASK <<
> +			EQOS_MAC_RXQ_CTRL0_RXQ0EN_SHIFT,
> +			0x2 <<
> +			EQOS_MAC_RXQ_CTRL0_RXQ0EN_SHIFT);
> +

This line just overrides the value set in the previous line.
Is it a mistake?

> +	/* enable promise mode */
> +	setbits_le32(&eqos->mac_regs->unused_004[1],
> +		     0x1);
> +

Isn't this mode also useful only for debugging?