Hi, Jakub,
Sorry for the late reply.
On Tue, Jan 28, 2025 at 6:06 AM Jakub Kicinski kuba@kernel.org wrote:
On Tue, 21 Jan 2025 17:37:03 +0800 Huacai Chen wrote:
Now for dwmac-loongson {tx,rx}_fifo_size are uninitialised, which means zero. This means dwmac-loongson doesn't support changing MTU,
Please provide more details here than "doesn't support changing". Does it return an error every time, but the device is operating correctly?
Do the flow control thresholds also get programmed incorrectly?
OK, I will give more details, the root cause is stmmac_change_mtu() fails if txfifosz is zero.
so set the correct tx_fifo_size and rx_fifo_size for it (16KB multiplied by channel counts).
Note: the Fixes tag is not exactly right, but it is a key commit of the dwmac-loongson series.
Please pick a better one, then. Oldest commit where issue can be observed by the user is usually a safe choice.
Emmm, the commit I picked is the oldest in the "stmmac: Add Loongson platform support".
Please use 12 chars of the hash in the tag.
OK, will do.
plat->tx_fifo_size = SZ_16K * plat->tx_queues_to_use;plat->rx_fifo_size = SZ_16K * plat->rx_queues_to_use;Is this really right? 16k times the number of queues seems like you're just trying to get the main driver to calculate 16k. What if user decreases the queue count? Maybe you should add a way to communicate the fifo size regardless of the queue count to the main driver?
Here {tx,rx}_fifo_size is initialised before stmmac_dvr_probe(), so it uses the maximum {tx,rx}_queues_to_use to calculate.
If we use ethtool to decrease queue count, stmmac_change_mtu() will get txfifosz larger than 16K, but stmmac_change_mtu() can still work because there is a condition "if ((txfifosz < new_mtu) || (new_mtu > BUF_SIZE_16KiB))".
This is not perfect, but it seems a perfect solution needs to rework the main driver, and dwmac-intel also uses {tx,rx}_queues_to_use to calculate {tx,rx}_fifo_size now.
Huacai
-- pw-bot: cr