netdev - Re: [PATCH net] Revert "net: stmmac: Specify hardware capability value when FIFO size isn't specified"

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4ed5b1dc-4f5c-4d2a-97e2-7b1f43dccfdc@arm.com>
Date: Thu, 6 Feb 2025 09:25:23 +0000
From: Steven Price <steven.price@....com>
To: "Russell King (Oracle)" <rmk+kernel@...linux.org.uk>,
 Andrew Lunn <andrew@...n.ch>, Heiner Kallweit <hkallweit1@...il.com>
Cc: Andrew Lunn <andrew+netdev@...n.ch>, "David S. Miller"
 <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
 Maxime Coquelin <mcoquelin.stm32@...il.com>,
 Alexandre Torgue <alexandre.torgue@...s.st.com>, netdev@...r.kernel.org,
 linux-stm32@...md-mailman.stormreply.com,
 linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH net] Revert "net: stmmac: Specify hardware capability
 value when FIFO size isn't specified"

On 05/02/2025 12:57, Russell King (Oracle) wrote:
> This reverts commit 8865d22656b4, which caused breakage for platforms
> which are not using xgmac2 or gmac4. Only these two cores have the
> capability of providing the FIFO sizes from hardware capability fields
> (which are provided in priv->dma_cap.[tr]x_fifo_size.)
> 
> All other cores can not, which results in these two fields containing
> zero. We also have platforms that do not provide a value in
> priv->plat->[tr]x_fifo_size, resulting in these also being zero.
> 
> This causes the new tests introduced by the reverted commit to fail,
> and produce e.g.:
> 
> 	stmmaceth f0804000.eth: Can't specify Rx FIFO size
> 
> An example of such a platform which fails is QEMU's npcm750-evb.
> This uses dwmac1000 which, as noted above, does not have the capability
> to provide the FIFO sizes from hardware.
> 
> Therefore, revert the commit to maintain compatibility with the way
> the driver used to work.
> 
> Reported-by: Guenter Roeck <linux@...ck-us.net>
> Link: https://lore.kernel.org/r/4e98f967-f636-46fb-9eca-d383b9495b86@roeck-us.net
> Signed-off-by: Russell King (Oracle) <rmk+kernel@...linux.org.uk>

Tested on my Firefly RK3288

Tested-by: Steven Price <steven.price@....com>

> ---
>  .../net/ethernet/stmicro/stmmac/stmmac_main.c | 35 +++++++++----------
>  1 file changed, 17 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index d04543e5697b..b34ebb916b89 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -2424,6 +2424,11 @@ static void stmmac_dma_operation_mode(struct stmmac_priv *priv)
>  	u32 chan = 0;
>  	u8 qmode = 0;
>  
> +	if (rxfifosz == 0)
> +		rxfifosz = priv->dma_cap.rx_fifo_size;
> +	if (txfifosz == 0)
> +		txfifosz = priv->dma_cap.tx_fifo_size;
> +
>  	/* Split up the shared Tx/Rx FIFO memory on DW QoS Eth and DW XGMAC */
>  	if (priv->plat->has_gmac4 || priv->plat->has_xgmac) {
>  		rxfifosz /= rx_channels_count;
> @@ -2892,6 +2897,11 @@ static void stmmac_set_dma_operation_mode(struct stmmac_priv *priv, u32 txmode,
>  	int rxfifosz = priv->plat->rx_fifo_size;
>  	int txfifosz = priv->plat->tx_fifo_size;
>  
> +	if (rxfifosz == 0)
> +		rxfifosz = priv->dma_cap.rx_fifo_size;
> +	if (txfifosz == 0)
> +		txfifosz = priv->dma_cap.tx_fifo_size;
> +
>  	/* Adjust for real per queue fifo size */
>  	rxfifosz /= rx_channels_count;
>  	txfifosz /= tx_channels_count;
> @@ -5868,6 +5878,9 @@ static int stmmac_change_mtu(struct net_device *dev, int new_mtu)
>  	const int mtu = new_mtu;
>  	int ret;
>  
> +	if (txfifosz == 0)
> +		txfifosz = priv->dma_cap.tx_fifo_size;
> +
>  	txfifosz /= priv->plat->tx_queues_to_use;
>  
>  	if (stmmac_xdp_is_enabled(priv) && new_mtu > ETH_DATA_LEN) {
> @@ -7219,29 +7232,15 @@ static int stmmac_hw_init(struct stmmac_priv *priv)
>  		priv->plat->tx_queues_to_use = priv->dma_cap.number_tx_queues;
>  	}
>  
> -	if (!priv->plat->rx_fifo_size) {
> -		if (priv->dma_cap.rx_fifo_size) {
> -			priv->plat->rx_fifo_size = priv->dma_cap.rx_fifo_size;
> -		} else {
> -			dev_err(priv->device, "Can't specify Rx FIFO size\n");
> -			return -ENODEV;
> -		}
> -	} else if (priv->dma_cap.rx_fifo_size &&
> -		   priv->plat->rx_fifo_size > priv->dma_cap.rx_fifo_size) {
> +	if (priv->dma_cap.rx_fifo_size &&
> +	    priv->plat->rx_fifo_size > priv->dma_cap.rx_fifo_size) {
>  		dev_warn(priv->device,
>  			 "Rx FIFO size (%u) exceeds dma capability\n",
>  			 priv->plat->rx_fifo_size);
>  		priv->plat->rx_fifo_size = priv->dma_cap.rx_fifo_size;
>  	}
> -	if (!priv->plat->tx_fifo_size) {
> -		if (priv->dma_cap.tx_fifo_size) {
> -			priv->plat->tx_fifo_size = priv->dma_cap.tx_fifo_size;
> -		} else {
> -			dev_err(priv->device, "Can't specify Tx FIFO size\n");
> -			return -ENODEV;
> -		}
> -	} else if (priv->dma_cap.tx_fifo_size &&
> -		   priv->plat->tx_fifo_size > priv->dma_cap.tx_fifo_size) {
> +	if (priv->dma_cap.tx_fifo_size &&
> +	    priv->plat->tx_fifo_size > priv->dma_cap.tx_fifo_size) {
>  		dev_warn(priv->device,
>  			 "Tx FIFO size (%u) exceeds dma capability\n",
>  			 priv->plat->tx_fifo_size);