Now for dwmac-loongson {tx,rx}_fifo_size are uninitialised, which means zero. This means dwmac-loongson doesn't support changing MTU, 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.
Cc: stable@vger.kernel.org Fixes: ad72f783de06827a1f ("net: stmmac: Add multi-channel support") Signed-off-by: Chong Qiao qiaochong@loongson.cn Signed-off-by: Huacai Chen chenhuacai@loongson.cn --- drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c index bfe6e2d631bd..79acdf38c525 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c @@ -574,6 +574,9 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id if (ret) goto err_disable_device;
+ plat->tx_fifo_size = SZ_16K * plat->tx_queues_to_use; + plat->rx_fifo_size = SZ_16K * plat->rx_queues_to_use; + if (dev_of_node(&pdev->dev)) ret = loongson_dwmac_dt_config(pdev, plat, &res); else
+ Feiyang Chen, Yanteng Si, Alexandre Torgue, Maxime Coquelin, Serge Semin, linux-arm-kernel
On Tue, Jan 21, 2025 at 05:37:03PM +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, 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.
Cc: stable@vger.kernel.org Fixes: ad72f783de06827a1f ("net: stmmac: Add multi-channel support") Signed-off-by: Chong Qiao qiaochong@loongson.cn Signed-off-by: Huacai Chen chenhuacai@loongson.cn
Thanks, this change looks good to me. And I agree that MTU setting cannot succeed without it.
Reviewed-by: Simon Horman horms@kernel.org
Some process notes regarding Networking patches to keep in mind for next time.
1. Please set the target tree. In this case, as this is a fix for code present in net. In general, otherwise it would be net-next.
Subject: [PATCH net] ...
2. Please generate a CC list using
./scripts/get_maintainer.pl this.patch
The b4 tool can help with this.
Link: https://docs.kernel.org/process/maintainer-netdev.html
drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c index bfe6e2d631bd..79acdf38c525 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c @@ -574,6 +574,9 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id if (ret) goto err_disable_device;
- plat->tx_fifo_size = SZ_16K * plat->tx_queues_to_use;
- plat->rx_fifo_size = SZ_16K * plat->rx_queues_to_use;
- if (dev_of_node(&pdev->dev)) ret = loongson_dwmac_dt_config(pdev, plat, &res); else
-- 2.47.1
On Wed, Jan 22, 2025 at 9:34 PM Simon Horman horms@kernel.org wrote:
- Feiyang Chen, Yanteng Si, Alexandre Torgue, Maxime Coquelin, Serge Semin, linux-arm-kernel
On Tue, Jan 21, 2025 at 05:37:03PM +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, 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.
Cc: stable@vger.kernel.org Fixes: ad72f783de06827a1f ("net: stmmac: Add multi-channel support") Signed-off-by: Chong Qiao qiaochong@loongson.cn Signed-off-by: Huacai Chen chenhuacai@loongson.cn
Thanks, this change looks good to me. And I agree that MTU setting cannot succeed without it.
Reviewed-by: Simon Horman horms@kernel.org
Some process notes regarding Networking patches to keep in mind for next time.
Please set the target tree. In this case, as this is a fix for code present in net. In general, otherwise it would be net-next.
Subject: [PATCH net] ...
Please generate a CC list using
./scripts/get_maintainer.pl this.patch
The b4 tool can help with this.
Link: https://docs.kernel.org/process/maintainer-netdev.html
OK, thanks.
Huacai
drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c index bfe6e2d631bd..79acdf38c525 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c @@ -574,6 +574,9 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id if (ret) goto err_disable_device;
plat->tx_fifo_size = SZ_16K * plat->tx_queues_to_use;plat->rx_fifo_size = SZ_16K * plat->rx_queues_to_use;if (dev_of_node(&pdev->dev)) ret = loongson_dwmac_dt_config(pdev, plat, &res); else-- 2.47.1
在 1/21/25 17:37, Huacai Chen 写道:
Now for dwmac-loongson {tx,rx}_fifo_size are uninitialised, which means zero. This means dwmac-loongson doesn't support changing MTU, 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.
Cc: stable@vger.kernel.org Fixes: ad72f783de06827a1f ("net: stmmac: Add multi-channel support") Signed-off-by: Chong Qiao qiaochong@loongson.cn Signed-off-by: Huacai Chen chenhuacai@loongson.cn
Acked-by: Yanteng Si si.yanteng@linux.dev
Thanks, Yanteng
drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c index bfe6e2d631bd..79acdf38c525 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c @@ -574,6 +574,9 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id if (ret) goto err_disable_device;
- plat->tx_fifo_size = SZ_16K * plat->tx_queues_to_use;
- plat->rx_fifo_size = SZ_16K * plat->rx_queues_to_use;
- if (dev_of_node(&pdev->dev)) ret = loongson_dwmac_dt_config(pdev, plat, &res); else
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?
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.
Please use 12 chars of the hash in the tag.
- 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?
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
linux-stable-mirror@lists.linaro.org
-
Huacai Chen -
Huacai Chen -
Jakub Kicinski -
Simon Horman -
Yanteng Si