[PATCH net-next 0/3] net: stmmac: xgmac: Minor fixes

[PATCH net-next 0/3] net: stmmac: xgmac: Minor fixes

[Rohan G Thomas via B4 Relay]

This patch series includes following minor fixes for stmmac
dwxgmac driver:

    1. Disable Rx FIFO overflow interrupt for dwxgmac
    2. Correct supported speed modes for dwxgmac
    3. Check for coe-unsupported flag before setting CIC bit of
       Tx Desc3 in the AF_XDP flow

Signed-off-by: Rohan G Thomas <rohan.g.thomas@altera.com>
---
Rohan G Thomas (3):
      net: stmmac: xgmac: Disable RX FIFO Overflow interrupts
      net: stmmac: xgmac: Correct supported speed modes
      net: stmmac: Set CIC bit only for TX queues with COE

 drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c | 14 ++++++++++++--
 drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c  |  5 +----
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c   |  6 ++++--
 3 files changed, 17 insertions(+), 8 deletions(-)
---
base-commit: b06c4311711c57c5e558bd29824b08f0a6e2a155
change-id: 20250714-xgmac-minor-fixes-40287f221dce

Best regards,
-- 
Rohan G Thomas <rohan.g.thomas@altera.com>

[PATCH net-next 1/3] net: stmmac: xgmac: Disable RX FIFO Overflow interrupts

[Rohan G Thomas via B4 Relay]

From: Rohan G Thomas <rohan.g.thomas@altera.com>

Enabling RX FIFO Overflow interrupts is counterproductive
and causes an interrupt storm when RX FIFO overflows.
Disabling this interrupt has no side effect and eliminates
interrupt storms when the RX FIFO overflows.

Commit 8a7cb245cf28 ("net: stmmac: Do not enable RX FIFO
overflow interrupts") disables RX FIFO overflow interrupts
for DWMAC4 IP and removes the corresponding handling of
this interrupt. This patch is doing the same thing for
XGMAC IP.

Signed-off-by: Rohan G Thomas <rohan.g.thomas@altera.com>
Reviewed-by: Matthew Gerlach <matthew.gerlach@altera.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
index 5dcc95bc0ad28b756accf9670c5fa00aa94fcfe3..7201a38842651a865493fce0cefe757d6ae9bafa 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
@@ -203,10 +203,6 @@ static void dwxgmac2_dma_rx_mode(struct stmmac_priv *priv, void __iomem *ioaddr,
 	}
 
 	writel(value, ioaddr + XGMAC_MTL_RXQ_OPMODE(channel));
-
-	/* Enable MTL RX overflow */
-	value = readl(ioaddr + XGMAC_MTL_QINTEN(channel));
-	writel(value | XGMAC_RXOIE, ioaddr + XGMAC_MTL_QINTEN(channel));
 }
 
 static void dwxgmac2_dma_tx_mode(struct stmmac_priv *priv, void __iomem *ioaddr,

-- 
2.25.1

[PATCH net-next 2/3] net: stmmac: xgmac: Correct supported speed modes

[Rohan G Thomas via B4 Relay]

From: Rohan G Thomas <rohan.g.thomas@altera.com>

Correct supported speed modes as per the XGMAC databook.
Commit 9cb54af214a7 ("net: stmmac: Fix IP-cores specific
MAC capabilities") removes support for 10M, 100M and
1000HD. 1000HD is not supported by XGMAC IP, but it does
support 10M and 100M FD mode, and it also supports 10M and
100M HD mode if the HDSEL bit is set in the MAC_HW_FEATURE0
reg. This commit adds support for 10M and 100M speed modes
for XGMAC IP.

Fixes: 9cb54af214a7 ("net: stmmac: Fix IP-cores specific MAC capabilities")
Signed-off-by: Rohan G Thomas <rohan.g.thomas@altera.com>
Reviewed-by: Matthew Gerlach <matthew.gerlach@altera.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c | 14 ++++++++++++--
 drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c  |  1 +
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
index 6cadf8de4fdfdb18af1a112b883b3d33a53da638..3cef8ba5a7f6c1b02881b8a4ac1eadb18ecfece4 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
@@ -49,6 +49,15 @@ static void dwxgmac2_core_init(struct mac_device_info *hw,
 	writel(XGMAC_INT_DEFAULT_EN, ioaddr + XGMAC_INT_EN);
 }
 
+static void dwxgmac2_update_caps(struct stmmac_priv *priv)
+{
+	/* If HDSEL bit is set in MAC_HW_Feature0 register then XGMAC supports
+	 * half duplex mode but only for 10Mbps and 100Mbps speed modes.
+	 */
+	if (priv->dma_cap.half_duplex)
+		priv->hw->link.caps |= (MAC_10HD | MAC_100HD);
+}
+
 static void dwxgmac2_set_mac(void __iomem *ioaddr, bool enable)
 {
 	u32 tx = readl(ioaddr + XGMAC_TX_CONFIG);
@@ -1424,6 +1433,7 @@ static void dwxgmac2_set_arp_offload(struct mac_device_info *hw, bool en,
 
 const struct stmmac_ops dwxgmac210_ops = {
 	.core_init = dwxgmac2_core_init,
+	.update_caps = dwxgmac2_update_caps,
 	.set_mac = dwxgmac2_set_mac,
 	.rx_ipc = dwxgmac2_rx_ipc,
 	.rx_queue_enable = dwxgmac2_rx_queue_enable,
@@ -1532,8 +1542,8 @@ int dwxgmac2_setup(struct stmmac_priv *priv)
 		mac->mcast_bits_log2 = ilog2(mac->multicast_filter_bins);
 
 	mac->link.caps = MAC_ASYM_PAUSE | MAC_SYM_PAUSE |
-			 MAC_1000FD | MAC_2500FD | MAC_5000FD |
-			 MAC_10000FD;
+			 MAC_10FD | MAC_100FD | MAC_1000FD |
+			 MAC_2500FD | MAC_5000FD | MAC_10000FD;
 	mac->link.duplex = 0;
 	mac->link.speed10 = XGMAC_CONFIG_SS_10_MII;
 	mac->link.speed100 = XGMAC_CONFIG_SS_100_MII;
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
index 7201a38842651a865493fce0cefe757d6ae9bafa..76a98d28f9de693ef6cb4c115e38f69c9a965b54 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
@@ -405,6 +405,7 @@ static int dwxgmac2_get_hw_feature(void __iomem *ioaddr,
 	dma_cap->sma_mdio = (hw_cap & XGMAC_HWFEAT_SMASEL) >> 5;
 	dma_cap->vlhash = (hw_cap & XGMAC_HWFEAT_VLHASH) >> 4;
 	dma_cap->half_duplex = (hw_cap & XGMAC_HWFEAT_HDSEL) >> 3;
+	dma_cap->mbps_10_100 = (hw_cap & XGMAC_HWFEAT_GMIISEL) >> 1;
 	dma_cap->mbps_1000 = (hw_cap & XGMAC_HWFEAT_GMIISEL) >> 1;
 
 	/* MAC HW feature 1 */

-- 
2.25.1

[PATCH net-next 3/3] net: stmmac: Set CIC bit only for TX queues with COE

[Rohan G Thomas via B4 Relay]

From: Rohan G Thomas <rohan.g.thomas@altera.com>

Currently, in the AF_XDP transmit paths, the CIC bit of
TX Desc3 is set for all packets. Setting this bit for
packets transmitting through queues that don't support
checksum offloading causes the TX DMA to get stuck after
transmitting some packets. This patch ensures the CIC bit
of TX Desc3 is set only if the TX queue supports checksum
offloading.

Signed-off-by: Rohan G Thomas <rohan.g.thomas@altera.com>
Reviewed-by: Matthew Gerlach <matthew.gerlach@altera.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index f350a6662880a230a32ad6785c475cce4e950322..d9f7435a44fac695899e0b4ffd0dc7851c4e759f 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -2584,6 +2584,7 @@ static bool stmmac_xdp_xmit_zc(struct stmmac_priv *priv, u32 queue, u32 budget)
 	struct netdev_queue *nq = netdev_get_tx_queue(priv->dev, queue);
 	struct stmmac_tx_queue *tx_q = &priv->dma_conf.tx_queue[queue];
 	struct stmmac_txq_stats *txq_stats = &priv->xstats.txq_stats[queue];
+	bool csum = !priv->plat->tx_queues_cfg[queue].coe_unsupported;
 	struct xsk_buff_pool *pool = tx_q->xsk_pool;
 	unsigned int entry = tx_q->cur_tx;
 	struct dma_desc *tx_desc = NULL;
@@ -2671,7 +2672,7 @@ static bool stmmac_xdp_xmit_zc(struct stmmac_priv *priv, u32 queue, u32 budget)
 		}
 
 		stmmac_prepare_tx_desc(priv, tx_desc, 1, xdp_desc.len,
-				       true, priv->mode, true, true,
+				       csum, priv->mode, true, true,
 				       xdp_desc.len);
 
 		stmmac_enable_dma_transmission(priv, priv->ioaddr, queue);
@@ -4983,6 +4984,7 @@ static int stmmac_xdp_xmit_xdpf(struct stmmac_priv *priv, int queue,
 {
 	struct stmmac_txq_stats *txq_stats = &priv->xstats.txq_stats[queue];
 	struct stmmac_tx_queue *tx_q = &priv->dma_conf.tx_queue[queue];
+	bool csum = !priv->plat->tx_queues_cfg[queue].coe_unsupported;
 	unsigned int entry = tx_q->cur_tx;
 	struct dma_desc *tx_desc;
 	dma_addr_t dma_addr;
@@ -5034,7 +5036,7 @@ static int stmmac_xdp_xmit_xdpf(struct stmmac_priv *priv, int queue,
 	stmmac_set_desc_addr(priv, tx_desc, dma_addr);
 
 	stmmac_prepare_tx_desc(priv, tx_desc, 1, xdpf->len,
-			       true, priv->mode, true, true,
+			       csum, priv->mode, true, true,
 			       xdpf->len);
 
 	tx_q->tx_count_frames++;

-- 
2.25.1

Re: [PATCH net-next 1/3] net: stmmac: xgmac: Disable RX FIFO Overflow interrupts

[Andrew Lunn]

On Mon, Jul 14, 2025 at 03:59:17PM +0800, Rohan G Thomas via B4 Relay wrote:
> From: Rohan G Thomas <rohan.g.thomas@altera.com>
> 
> Enabling RX FIFO Overflow interrupts is counterproductive
> and causes an interrupt storm when RX FIFO overflows.
> Disabling this interrupt has no side effect and eliminates
> interrupt storms when the RX FIFO overflows.
> 
> Commit 8a7cb245cf28 ("net: stmmac: Do not enable RX FIFO
> overflow interrupts") disables RX FIFO overflow interrupts
> for DWMAC4 IP and removes the corresponding handling of
> this interrupt. This patch is doing the same thing for
> XGMAC IP.
> 
> Signed-off-by: Rohan G Thomas <rohan.g.thomas@altera.com>
> Reviewed-by: Matthew Gerlach <matthew.gerlach@altera.com>

Please take a read of:

https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html

This appears to be a fixed, so the Subject: line should indicate this.
Please also include a Fixes: tag, and Cc: stable.

> ---
>  drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
> index 5dcc95bc0ad28b756accf9670c5fa00aa94fcfe3..7201a38842651a865493fce0cefe757d6ae9bafa 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
> @@ -203,10 +203,6 @@ static void dwxgmac2_dma_rx_mode(struct stmmac_priv *priv, void __iomem *ioaddr,
>  	}
>  
>  	writel(value, ioaddr + XGMAC_MTL_RXQ_OPMODE(channel));
> -
> -	/* Enable MTL RX overflow */
> -	value = readl(ioaddr + XGMAC_MTL_QINTEN(channel));
> -	writel(value | XGMAC_RXOIE, ioaddr + XGMAC_MTL_QINTEN(channel));

What is the reset default? Would it make sense to explicitly disable
it, rather than never enable it? What does 8a7cb245cf28 do?

    Andrew

---
pw-bot: cr

Re: [PATCH net-next 3/3] net: stmmac: Set CIC bit only for TX queues with COE

[Simon Horman]

On Mon, Jul 14, 2025 at 03:59:19PM +0800, Rohan G Thomas via B4 Relay wrote:
> From: Rohan G Thomas <rohan.g.thomas@altera.com>
> 
> Currently, in the AF_XDP transmit paths, the CIC bit of
> TX Desc3 is set for all packets. Setting this bit for
> packets transmitting through queues that don't support
> checksum offloading causes the TX DMA to get stuck after
> transmitting some packets. This patch ensures the CIC bit
> of TX Desc3 is set only if the TX queue supports checksum
> offloading.
> 
> Signed-off-by: Rohan G Thomas <rohan.g.thomas@altera.com>
> Reviewed-by: Matthew Gerlach <matthew.gerlach@altera.com>

Hi Rohan,

I notice that stmmac_xmit() handles a few other cases where
checksum offload should not be requested via stmmac_prepare_tx_desc:

        csum_insertion = (skb->ip_summed == CHECKSUM_PARTIAL);
        /* DWMAC IPs can be synthesized to support tx coe only for a few tx
         * queues. In that case, checksum offloading for those queues that don't
         * support tx coe needs to fallback to software checksum calculation.
         *
         * Packets that won't trigger the COE e.g. most DSA-tagged packets will
         * also have to be checksummed in software.
         */
        if (csum_insertion &&
            (priv->plat->tx_queues_cfg[queue].coe_unsupported ||
             !stmmac_has_ip_ethertype(skb))) {
                if (unlikely(skb_checksum_help(skb)))
                        goto dma_map_err;
                csum_insertion = !csum_insertion;
        }

Do we need to care about them in stmmac_xdp_xmit_zc()
and stmmac_xdp_xmit_xdpf() too?

...

Re: [PATCH net-next 2/3] net: stmmac: xgmac: Correct supported speed modes

[Andrew Lunn]

On Mon, Jul 14, 2025 at 03:59:18PM +0800, Rohan G Thomas via B4 Relay wrote:
> From: Rohan G Thomas <rohan.g.thomas@altera.com>
> 
> Correct supported speed modes as per the XGMAC databook.
> Commit 9cb54af214a7 ("net: stmmac: Fix IP-cores specific
> MAC capabilities") removes support for 10M, 100M and
> 1000HD. 1000HD is not supported by XGMAC IP, but it does
> support 10M and 100M FD mode, and it also supports 10M and
> 100M HD mode if the HDSEL bit is set in the MAC_HW_FEATURE0
> reg. This commit adds support for 10M and 100M speed modes
> for XGMAC IP.

> +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
> @@ -405,6 +405,7 @@ static int dwxgmac2_get_hw_feature(void __iomem *ioaddr,
>  	dma_cap->sma_mdio = (hw_cap & XGMAC_HWFEAT_SMASEL) >> 5;
>  	dma_cap->vlhash = (hw_cap & XGMAC_HWFEAT_VLHASH) >> 4;
>  	dma_cap->half_duplex = (hw_cap & XGMAC_HWFEAT_HDSEL) >> 3;
> +	dma_cap->mbps_10_100 = (hw_cap & XGMAC_HWFEAT_GMIISEL) >> 1;

The commit message does not mention this change.

What does XGMAC_HWFEAT_GMIISEL mean? That a SERDES style interface is
not being used? Could that be why Serge removed these speeds? He was
looking at systems with a SERDES, and they don't support slower
speeds?

	Andrew

Re: [PATCH net-next 1/3] net: stmmac: xgmac: Disable RX FIFO Overflow interrupts

[G Thomas, Rohan]

Hi Andrew,

Thanks for reviewing the patch.

On 7/14/2025 7:04 PM, Andrew Lunn wrote:
> On Mon, Jul 14, 2025 at 03:59:17PM +0800, Rohan G Thomas via B4 Relay wrote:
>> From: Rohan G Thomas <rohan.g.thomas@altera.com>
>>
>> Enabling RX FIFO Overflow interrupts is counterproductive
>> and causes an interrupt storm when RX FIFO overflows.
>> Disabling this interrupt has no side effect and eliminates
>> interrupt storms when the RX FIFO overflows.
>>
>> Commit 8a7cb245cf28 ("net: stmmac: Do not enable RX FIFO
>> overflow interrupts") disables RX FIFO overflow interrupts
>> for DWMAC4 IP and removes the corresponding handling of
>> this interrupt. This patch is doing the same thing for
>> XGMAC IP.
>>
>> Signed-off-by: Rohan G Thomas <rohan.g.thomas@altera.com>
>> Reviewed-by: Matthew Gerlach <matthew.gerlach@altera.com>
> 
> Please take a read of:
> 
> https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html
> 
> This appears to be a fixed, so the Subject: line should indicate this.
> Please also include a Fixes: tag, and Cc: stable.

Agreed. Will do in the next version.

> 
>> ---
>>   drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c | 4 ----
>>   1 file changed, 4 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
>> index 5dcc95bc0ad28b756accf9670c5fa00aa94fcfe3..7201a38842651a865493fce0cefe757d6ae9bafa 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
>> @@ -203,10 +203,6 @@ static void dwxgmac2_dma_rx_mode(struct stmmac_priv *priv, void __iomem *ioaddr,
>>   	}
>>   
>>   	writel(value, ioaddr + XGMAC_MTL_RXQ_OPMODE(channel));
>> -
>> -	/* Enable MTL RX overflow */
>> -	value = readl(ioaddr + XGMAC_MTL_QINTEN(channel));
>> -	writel(value | XGMAC_RXOIE, ioaddr + XGMAC_MTL_QINTEN(channel));
> 
> What is the reset default? Would it make sense to explicitly disable
> it, rather than never enable it? What does 8a7cb245cf28 do?

The RX FIFO Overflow interrupt is disabled by default on reset. Commit
8a7cb245cf28 also avoids enabling the interrupt rather than disabling
it. This commit mirrors the same thing for the XGMAC IP.

> 
>      Andrew
> 
> ---
> pw-bot: cr
> 
> 

Best Regards,
Rohan

Re: [PATCH net-next 1/3] net: stmmac: xgmac: Disable RX FIFO Overflow interrupts

[Andrew Lunn]

> > What is the reset default? Would it make sense to explicitly disable
> > it, rather than never enable it? What does 8a7cb245cf28 do?
> 
> The RX FIFO Overflow interrupt is disabled by default on reset. Commit
> 8a7cb245cf28 also avoids enabling the interrupt rather than disabling
> it. This commit mirrors the same thing for the XGMAC IP.

So same as 8a7cb245cf28 is good.

When you resubmit with the correct Subject: please add Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net-next 2/3] net: stmmac: xgmac: Correct supported speed modes

[G Thomas, Rohan]

Hi Andrew,

Thanks for reviewing the patch.

On 7/14/2025 7:12 PM, Andrew Lunn wrote:
> On Mon, Jul 14, 2025 at 03:59:18PM +0800, Rohan G Thomas via B4 Relay wrote:
>> From: Rohan G Thomas <rohan.g.thomas@altera.com>
>>
>> Correct supported speed modes as per the XGMAC databook.
>> Commit 9cb54af214a7 ("net: stmmac: Fix IP-cores specific
>> MAC capabilities") removes support for 10M, 100M and
>> 1000HD. 1000HD is not supported by XGMAC IP, but it does
>> support 10M and 100M FD mode, and it also supports 10M and
>> 100M HD mode if the HDSEL bit is set in the MAC_HW_FEATURE0
>> reg. This commit adds support for 10M and 100M speed modes
>> for XGMAC IP.
> 
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
>> @@ -405,6 +405,7 @@ static int dwxgmac2_get_hw_feature(void __iomem *ioaddr,
>>   	dma_cap->sma_mdio = (hw_cap & XGMAC_HWFEAT_SMASEL) >> 5;
>>   	dma_cap->vlhash = (hw_cap & XGMAC_HWFEAT_VLHASH) >> 4;
>>   	dma_cap->half_duplex = (hw_cap & XGMAC_HWFEAT_HDSEL) >> 3;
>> +	dma_cap->mbps_10_100 = (hw_cap & XGMAC_HWFEAT_GMIISEL) >> 1;
> 
> The commit message does not mention this change.

Agreed. Will do in the next version.

> 
> What does XGMAC_HWFEAT_GMIISEL mean? That a SERDES style interface is
> not being used? Could that be why Serge removed these speeds? He was
> looking at systems with a SERDES, and they don't support slower
> speeds?
> 
> 	Andrew
As per the XGMAC databook ver 3.10a, GMIISEL bit of MAC_HW_Feature_0
register indicates whether the XGMAC IP on the SOC is synthesized with
DWCXG_GMII_SUPPORT. Specifically, it states:
"1000/100/10 Mbps Support. This bit is set to 1 when the GMII Interface
option is selected."

So yes, it’s likely that Serge was working with a SERDES interface which
doesn't support 10/100Mbps speeds. Do you think it would be appropriate
to add a check for this bit before enabling 10/100Mbps speeds?

Best Regards,
Rohan

Re: [PATCH net-next 3/3] net: stmmac: Set CIC bit only for TX queues with COE

[G Thomas, Rohan]

Hi Simon,

Thanks for reviewing the patch.

On 7/14/2025 7:10 PM, Simon Horman wrote:
> On Mon, Jul 14, 2025 at 03:59:19PM +0800, Rohan G Thomas via B4 Relay wrote:
>> From: Rohan G Thomas <rohan.g.thomas@altera.com>
>>
>> Currently, in the AF_XDP transmit paths, the CIC bit of
>> TX Desc3 is set for all packets. Setting this bit for
>> packets transmitting through queues that don't support
>> checksum offloading causes the TX DMA to get stuck after
>> transmitting some packets. This patch ensures the CIC bit
>> of TX Desc3 is set only if the TX queue supports checksum
>> offloading.
>>
>> Signed-off-by: Rohan G Thomas <rohan.g.thomas@altera.com>
>> Reviewed-by: Matthew Gerlach <matthew.gerlach@altera.com>
> 
> Hi Rohan,
> 
> I notice that stmmac_xmit() handles a few other cases where
> checksum offload should not be requested via stmmac_prepare_tx_desc:
> 
>          csum_insertion = (skb->ip_summed == CHECKSUM_PARTIAL);
>          /* DWMAC IPs can be synthesized to support tx coe only for a few tx
>           * queues. In that case, checksum offloading for those queues that don't
>           * support tx coe needs to fallback to software checksum calculation.
>           *
>           * Packets that won't trigger the COE e.g. most DSA-tagged packets will
>           * also have to be checksummed in software.
>           */
>          if (csum_insertion &&
>              (priv->plat->tx_queues_cfg[queue].coe_unsupported ||
>               !stmmac_has_ip_ethertype(skb))) {
>                  if (unlikely(skb_checksum_help(skb)))
>                          goto dma_map_err;
>                  csum_insertion = !csum_insertion;
>          }
> 
> Do we need to care about them in stmmac_xdp_xmit_zc()
> and stmmac_xdp_xmit_xdpf() too?

This patch only addresses avoiding the TX DMA hang by ensuring the CIC
bit is only set when the queue supports checksum offload. For DSA tagged
packets checksum offloading is not supported by the DWMAC IPs but no TX
DMA hang. AFAIK, currently AF_XDP paths don't have equivalent handling
like skb_checksum_help(), since they operate on xdp buffers. So this
patch doesn't attempt to implement a sw fallback but just avoids DMA
stall.

> 
> ...

Best Regards,
Rohan

Re: [PATCH net-next 2/3] net: stmmac: xgmac: Correct supported speed modes

[Andrew Lunn]

> As per the XGMAC databook ver 3.10a, GMIISEL bit of MAC_HW_Feature_0
> register indicates whether the XGMAC IP on the SOC is synthesized with
> DWCXG_GMII_SUPPORT. Specifically, it states:
> "1000/100/10 Mbps Support. This bit is set to 1 when the GMII Interface
> option is selected."
> 
> So yes, it’s likely that Serge was working with a SERDES interface which
> doesn't support 10/100Mbps speeds. Do you think it would be appropriate
> to add a check for this bit before enabling 10/100Mbps speeds?

Yes.

That is the problem with stuff you can synthesizer. You have no idea
what it actually is unless you read all the self enumerating
registers. Flexibility at the cost of complexity.

	Andrew

Re: [PATCH net-next 3/3] net: stmmac: Set CIC bit only for TX queues with COE

[Simon Horman]

On Tue, Jul 15, 2025 at 07:14:21PM +0530, G Thomas, Rohan wrote:
> Hi Simon,
> 
> Thanks for reviewing the patch.
> 
> On 7/14/2025 7:10 PM, Simon Horman wrote:
> > On Mon, Jul 14, 2025 at 03:59:19PM +0800, Rohan G Thomas via B4 Relay wrote:
> > > From: Rohan G Thomas <rohan.g.thomas@altera.com>
> > > 
> > > Currently, in the AF_XDP transmit paths, the CIC bit of
> > > TX Desc3 is set for all packets. Setting this bit for
> > > packets transmitting through queues that don't support
> > > checksum offloading causes the TX DMA to get stuck after
> > > transmitting some packets. This patch ensures the CIC bit
> > > of TX Desc3 is set only if the TX queue supports checksum
> > > offloading.
> > > 
> > > Signed-off-by: Rohan G Thomas <rohan.g.thomas@altera.com>
> > > Reviewed-by: Matthew Gerlach <matthew.gerlach@altera.com>
> > 
> > Hi Rohan,
> > 
> > I notice that stmmac_xmit() handles a few other cases where
> > checksum offload should not be requested via stmmac_prepare_tx_desc:
> > 
> >          csum_insertion = (skb->ip_summed == CHECKSUM_PARTIAL);
> >          /* DWMAC IPs can be synthesized to support tx coe only for a few tx
> >           * queues. In that case, checksum offloading for those queues that don't
> >           * support tx coe needs to fallback to software checksum calculation.
> >           *
> >           * Packets that won't trigger the COE e.g. most DSA-tagged packets will
> >           * also have to be checksummed in software.
> >           */
> >          if (csum_insertion &&
> >              (priv->plat->tx_queues_cfg[queue].coe_unsupported ||
> >               !stmmac_has_ip_ethertype(skb))) {
> >                  if (unlikely(skb_checksum_help(skb)))
> >                          goto dma_map_err;
> >                  csum_insertion = !csum_insertion;
> >          }
> > 
> > Do we need to care about them in stmmac_xdp_xmit_zc()
> > and stmmac_xdp_xmit_xdpf() too?
> 
> This patch only addresses avoiding the TX DMA hang by ensuring the CIC
> bit is only set when the queue supports checksum offload. For DSA tagged
> packets checksum offloading is not supported by the DWMAC IPs but no TX
> DMA hang. AFAIK, currently AF_XDP paths don't have equivalent handling
> like skb_checksum_help(), since they operate on xdp buffers. So this
> patch doesn't attempt to implement a sw fallback but just avoids DMA
> stall.

Ok, fair enough.

As per Andrew's advice elsewhere in this thread.
This patch also looks like it should be a fix for net,
and should have a Fixes tag.

Re: [PATCH net-next 3/3] net: stmmac: Set CIC bit only for TX queues with COE

[G Thomas, Rohan]

Hi Simon,

On 7/16/2025 1:52 PM, Simon Horman wrote:
> On Tue, Jul 15, 2025 at 07:14:21PM +0530, G Thomas, Rohan wrote:
>> Hi Simon,
>>
>> Thanks for reviewing the patch.
>>
>> On 7/14/2025 7:10 PM, Simon Horman wrote:
>>> On Mon, Jul 14, 2025 at 03:59:19PM +0800, Rohan G Thomas via B4 Relay wrote:
>>>> From: Rohan G Thomas <rohan.g.thomas@altera.com>
>>>>
>>>> Currently, in the AF_XDP transmit paths, the CIC bit of
>>>> TX Desc3 is set for all packets. Setting this bit for
>>>> packets transmitting through queues that don't support
>>>> checksum offloading causes the TX DMA to get stuck after
>>>> transmitting some packets. This patch ensures the CIC bit
>>>> of TX Desc3 is set only if the TX queue supports checksum
>>>> offloading.
>>>>
>>>> Signed-off-by: Rohan G Thomas <rohan.g.thomas@altera.com>
>>>> Reviewed-by: Matthew Gerlach <matthew.gerlach@altera.com>
>>>
>>> Hi Rohan,
>>>
>>> I notice that stmmac_xmit() handles a few other cases where
>>> checksum offload should not be requested via stmmac_prepare_tx_desc:
>>>
>>>           csum_insertion = (skb->ip_summed == CHECKSUM_PARTIAL);
>>>           /* DWMAC IPs can be synthesized to support tx coe only for a few tx
>>>            * queues. In that case, checksum offloading for those queues that don't
>>>            * support tx coe needs to fallback to software checksum calculation.
>>>            *
>>>            * Packets that won't trigger the COE e.g. most DSA-tagged packets will
>>>            * also have to be checksummed in software.
>>>            */
>>>           if (csum_insertion &&
>>>               (priv->plat->tx_queues_cfg[queue].coe_unsupported ||
>>>                !stmmac_has_ip_ethertype(skb))) {
>>>                   if (unlikely(skb_checksum_help(skb)))
>>>                           goto dma_map_err;
>>>                   csum_insertion = !csum_insertion;
>>>           }
>>>
>>> Do we need to care about them in stmmac_xdp_xmit_zc()
>>> and stmmac_xdp_xmit_xdpf() too?
>>
>> This patch only addresses avoiding the TX DMA hang by ensuring the CIC
>> bit is only set when the queue supports checksum offload. For DSA tagged
>> packets checksum offloading is not supported by the DWMAC IPs but no TX
>> DMA hang. AFAIK, currently AF_XDP paths don't have equivalent handling
>> like skb_checksum_help(), since they operate on xdp buffers. So this
>> patch doesn't attempt to implement a sw fallback but just avoids DMA
>> stall.
> 
> Ok, fair enough.
> 
> As per Andrew's advice elsewhere in this thread.
> This patch also looks like it should be a fix for net,
> and should have a Fixes tag.

Thanks for your comments.

You're right—this patch is a fix for the TX DMA hang issue caused by
setting the CIC bit on queues that don't support checksum offload. But
I couldn’t pinpoint a specific commit that introduced this behavior in
the AF_XDP path. Initially, there was no support for DWMAC IPs with COE
enabled only on specific queues, even though there can be IPs with such
configuration. Commit 8452a05b2c63 ("net: stmmac: Tx coe sw fallback")
added software fallback support for the AF_PACKET path. But the AF_XDP
path has always enabled COE unconditionally even before that. So, do you
think referencing the commit 8452a05b2c63 in the Fixes tag is
appropriate and sufficient?

Best Regards,
Rohan

Re: [PATCH net-next 2/3] net: stmmac: xgmac: Correct supported speed modes

[Russell King (Oracle)]

On Mon, Jul 14, 2025 at 03:59:18PM +0800, Rohan G Thomas via B4 Relay wrote:
> @@ -1532,8 +1542,8 @@ int dwxgmac2_setup(struct stmmac_priv *priv)
>  		mac->mcast_bits_log2 = ilog2(mac->multicast_filter_bins);
>  
>  	mac->link.caps = MAC_ASYM_PAUSE | MAC_SYM_PAUSE |
> -			 MAC_1000FD | MAC_2500FD | MAC_5000FD |
> -			 MAC_10000FD;
> +			 MAC_10FD | MAC_100FD | MAC_1000FD |
> +			 MAC_2500FD | MAC_5000FD | MAC_10000FD;
...
> @@ -405,6 +405,7 @@ static int dwxgmac2_get_hw_feature(void __iomem *ioaddr,
>  	dma_cap->sma_mdio = (hw_cap & XGMAC_HWFEAT_SMASEL) >> 5;
>  	dma_cap->vlhash = (hw_cap & XGMAC_HWFEAT_VLHASH) >> 4;
>  	dma_cap->half_duplex = (hw_cap & XGMAC_HWFEAT_HDSEL) >> 3;
> +	dma_cap->mbps_10_100 = (hw_cap & XGMAC_HWFEAT_GMIISEL) >> 1;

What if dma_cap->mbps_10_100 is false? Should MAC_10FD | MAC_100FD
still be set? What if dma_cap->half_duplex is set but
dma_cap->mbps_10_100 is not? Should we avoid setting 10HD and 100HD?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next 2/3] net: stmmac: xgmac: Correct supported speed modes

[Serge Semin]

On Tue, Jul 15, 2025 at 07:03:58PM +0530, G Thomas, Rohan wrote:
> Hi Andrew,
> 
> Thanks for reviewing the patch.
> 
> On 7/14/2025 7:12 PM, Andrew Lunn wrote:
> > On Mon, Jul 14, 2025 at 03:59:18PM +0800, Rohan G Thomas via B4 Relay wrote:
> > > From: Rohan G Thomas <rohan.g.thomas@altera.com>
> > > 
> > > Correct supported speed modes as per the XGMAC databook.
> > > Commit 9cb54af214a7 ("net: stmmac: Fix IP-cores specific
> > > MAC capabilities") removes support for 10M, 100M and
> > > 1000HD. 1000HD is not supported by XGMAC IP, but it does
> > > support 10M and 100M FD mode, and it also supports 10M and
> > > 100M HD mode if the HDSEL bit is set in the MAC_HW_FEATURE0
> > > reg. This commit adds support for 10M and 100M speed modes
> > > for XGMAC IP.
> > 
> > > +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
> > > @@ -405,6 +405,7 @@ static int dwxgmac2_get_hw_feature(void __iomem *ioaddr,
> > >   	dma_cap->sma_mdio = (hw_cap & XGMAC_HWFEAT_SMASEL) >> 5;
> > >   	dma_cap->vlhash = (hw_cap & XGMAC_HWFEAT_VLHASH) >> 4;
> > >   	dma_cap->half_duplex = (hw_cap & XGMAC_HWFEAT_HDSEL) >> 3;
> > > +	dma_cap->mbps_10_100 = (hw_cap & XGMAC_HWFEAT_GMIISEL) >> 1;
> > 
> > The commit message does not mention this change.
> 
> Agreed. Will do in the next version.
> 
> > 
> > What does XGMAC_HWFEAT_GMIISEL mean? That a SERDES style interface is
> > not being used? Could that be why Serge removed these speeds? He was
> > looking at systems with a SERDES, and they don't support slower
> > speeds?
> > 
> > 	Andrew
> As per the XGMAC databook ver 3.10a, GMIISEL bit of MAC_HW_Feature_0
> register indicates whether the XGMAC IP on the SOC is synthesized with
> DWCXG_GMII_SUPPORT. Specifically, it states:
> "1000/100/10 Mbps Support. This bit is set to 1 when the GMII Interface
> option is selected."
> 
> So yes, it’s likely that Serge was working with a SERDES interface which
> doesn't support 10/100Mbps speeds. Do you think it would be appropriate
> to add a check for this bit before enabling 10/100Mbps speeds?

DW XGMAC IP-core of v2.x and older don't support 10/100Mbps modes
neither in the XGMII nor in the GMII interfaces. That's why I dropped
the 10/100Mbps link capabilities retaining 1G, 2.5G and 10G speeds
only (the only speeds supported for DW XGMAC 1.20a/2.11a Tx in the
MAC_Tx_Configuration.SS register field). Although I should have
dropped the MAC_5000FD too since it has been supported since v3.0
IP-core version. My bad.(

Starting from DW XGMAC v3.00a IP-core the list of the supported speeds
has been extended to: 10/100Mbps (MII), 1G/2.5G (GMII), 2.5G/5G/10G
(XGMII). Thus the more appropriate fix here should take into account
the IP-core version. Like this:
	if (dma_cap->mbps_1000 && MAC_Version.SNPSVER >= 0x30)
		dma_cap->mbps_10_100 = 1;

Then you can use the mbps_1000 and mbps_10_100 flags to set the proper
MAC-capabilities to hw->link.caps in the dwxgmac2_setup() method. I
would have added the XGMII 2.5G/5G MAC-capabilities setting up to the
dwxgmac2_setup() method too for the v3.x IP-cores and newer.

-Serge(y)

> 
> Best Regards,
> Rohan
> 

Re: [PATCH net-next 2/3] net: stmmac: xgmac: Correct supported speed modes

[G Thomas, Rohan]

Hi Russell,

Thanks for reviewing the patch.

On 7/17/2025 12:15 PM, Russell King (Oracle) wrote:
> On Mon, Jul 14, 2025 at 03:59:18PM +0800, Rohan G Thomas via B4 Relay wrote:
>> @@ -1532,8 +1542,8 @@ int dwxgmac2_setup(struct stmmac_priv *priv)
>>   		mac->mcast_bits_log2 = ilog2(mac->multicast_filter_bins);
>>   
>>   	mac->link.caps = MAC_ASYM_PAUSE | MAC_SYM_PAUSE |
>> -			 MAC_1000FD | MAC_2500FD | MAC_5000FD |
>> -			 MAC_10000FD;
>> +			 MAC_10FD | MAC_100FD | MAC_1000FD |
>> +			 MAC_2500FD | MAC_5000FD | MAC_10000FD;
> ...
>> @@ -405,6 +405,7 @@ static int dwxgmac2_get_hw_feature(void __iomem *ioaddr,
>>   	dma_cap->sma_mdio = (hw_cap & XGMAC_HWFEAT_SMASEL) >> 5;
>>   	dma_cap->vlhash = (hw_cap & XGMAC_HWFEAT_VLHASH) >> 4;
>>   	dma_cap->half_duplex = (hw_cap & XGMAC_HWFEAT_HDSEL) >> 3;
>> +	dma_cap->mbps_10_100 = (hw_cap & XGMAC_HWFEAT_GMIISEL) >> 1;
> 
> What if dma_cap->mbps_10_100 is false? Should MAC_10FD | MAC_100FD
> still be set? What if dma_cap->half_duplex is set but
> dma_cap->mbps_10_100 is not? Should we avoid setting 10HD and 100HD?

As per the XGMAC databook, 10Mbps/100Mbps/1Gbps speeds are supported
only when the GMIISEL bit is set. As Serge pointed out, I also need to
consider the MAC version (≥ v3.00a) when enabling these modes. I’ll
update the next version of the patch to include checks for both the
GMIISEL bit and the MAC version before enabling the 
MAC_10FD/MAC_100FD/MAC_1000FD capabilities.

Also, regarding the HDSEL bit — it is set only if 10Mbps/100Mbps modes
are supported. I’ll include this condition as well when handling half
duplex support in the updated patch.

> 

Best Regards,
Rohan

Re: [PATCH net-next 2/3] net: stmmac: xgmac: Correct supported speed modes

[G Thomas, Rohan]

Hi Serge,

Thanks for the review comments and the detailed explanation.

On 7/17/2025 5:17 PM, Serge Semin wrote:
> On Tue, Jul 15, 2025 at 07:03:58PM +0530, G Thomas, Rohan wrote:
>> Hi Andrew,
>>
>> Thanks for reviewing the patch.
>>
>> On 7/14/2025 7:12 PM, Andrew Lunn wrote:
>>> On Mon, Jul 14, 2025 at 03:59:18PM +0800, Rohan G Thomas via B4 Relay wrote:
>>>> From: Rohan G Thomas <rohan.g.thomas@altera.com>
>>>>
>>>> Correct supported speed modes as per the XGMAC databook.
>>>> Commit 9cb54af214a7 ("net: stmmac: Fix IP-cores specific
>>>> MAC capabilities") removes support for 10M, 100M and
>>>> 1000HD. 1000HD is not supported by XGMAC IP, but it does
>>>> support 10M and 100M FD mode, and it also supports 10M and
>>>> 100M HD mode if the HDSEL bit is set in the MAC_HW_FEATURE0
>>>> reg. This commit adds support for 10M and 100M speed modes
>>>> for XGMAC IP.
>>>
>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
>>>> @@ -405,6 +405,7 @@ static int dwxgmac2_get_hw_feature(void __iomem *ioaddr,
>>>>    	dma_cap->sma_mdio = (hw_cap & XGMAC_HWFEAT_SMASEL) >> 5;
>>>>    	dma_cap->vlhash = (hw_cap & XGMAC_HWFEAT_VLHASH) >> 4;
>>>>    	dma_cap->half_duplex = (hw_cap & XGMAC_HWFEAT_HDSEL) >> 3;
>>>> +	dma_cap->mbps_10_100 = (hw_cap & XGMAC_HWFEAT_GMIISEL) >> 1;
>>>
>>> The commit message does not mention this change.
>>
>> Agreed. Will do in the next version.
>>
>>>
>>> What does XGMAC_HWFEAT_GMIISEL mean? That a SERDES style interface is
>>> not being used? Could that be why Serge removed these speeds? He was
>>> looking at systems with a SERDES, and they don't support slower
>>> speeds?
>>>
>>> 	Andrew
>> As per the XGMAC databook ver 3.10a, GMIISEL bit of MAC_HW_Feature_0
>> register indicates whether the XGMAC IP on the SOC is synthesized with
>> DWCXG_GMII_SUPPORT. Specifically, it states:
>> "1000/100/10 Mbps Support. This bit is set to 1 when the GMII Interface
>> option is selected."
>>
>> So yes, it’s likely that Serge was working with a SERDES interface which
>> doesn't support 10/100Mbps speeds. Do you think it would be appropriate
>> to add a check for this bit before enabling 10/100Mbps speeds?
> 
> DW XGMAC IP-core of v2.x and older don't support 10/100Mbps modes
> neither in the XGMII nor in the GMII interfaces. That's why I dropped
> the 10/100Mbps link capabilities retaining 1G, 2.5G and 10G speeds
> only (the only speeds supported for DW XGMAC 1.20a/2.11a Tx in the
> MAC_Tx_Configuration.SS register field). Although I should have
> dropped the MAC_5000FD too since it has been supported since v3.0
> IP-core version. My bad.(
> 
> Starting from DW XGMAC v3.00a IP-core the list of the supported speeds
> has been extended to: 10/100Mbps (MII), 1G/2.5G (GMII), 2.5G/5G/10G
> (XGMII). Thus the more appropriate fix here should take into account
> the IP-core version. Like this:
> 	if (dma_cap->mbps_1000 && MAC_Version.SNPSVER >= 0x30)
> 		dma_cap->mbps_10_100 = 1;
>  > Then you can use the mbps_1000 and mbps_10_100 flags to set the proper
> MAC-capabilities to hw->link.caps in the dwxgmac2_setup() method. I
> would have added the XGMII 2.5G/5G MAC-capabilities setting up to the
> dwxgmac2_setup() method too for the v3.x IP-cores and newer.
> 

Agreed. Will do in the next version.

Will ensure that mbps_10_100 is set only if SNPSVER >= 0x30 and will
also conditionally enable 2.5G/5G MAC capabilities for IP versions
v3.00a and above.

In the stmmac_dvr_probe() the setup() callback is invoked before
hw_cap_support is populated. Given that, do you think it is sufficient
to add these checks into the dwxgmac2_update_caps() instead?

> -Serge(y)
> 
>>
>> Best Regards,
>> Rohan
>>

Best Regards,
Rohan

Re: [PATCH net-next 2/3] net: stmmac: xgmac: Correct supported speed modes

[Serge Semin]

On Thu, Jul 17, 2025 at 06:29:33PM +0530, G Thomas, Rohan wrote:
> Hi Serge,
> 
> Thanks for the review comments and the detailed explanation.
> 
> On 7/17/2025 5:17 PM, Serge Semin wrote:
> > On Tue, Jul 15, 2025 at 07:03:58PM +0530, G Thomas, Rohan wrote:

...

> > > 
> > > > 
> > > > What does XGMAC_HWFEAT_GMIISEL mean? That a SERDES style interface is
> > > > not being used? Could that be why Serge removed these speeds? He was
> > > > looking at systems with a SERDES, and they don't support slower
> > > > speeds?
> > > > 
> > > > 	Andrew
> > > As per the XGMAC databook ver 3.10a, GMIISEL bit of MAC_HW_Feature_0
> > > register indicates whether the XGMAC IP on the SOC is synthesized with
> > > DWCXG_GMII_SUPPORT. Specifically, it states:
> > > "1000/100/10 Mbps Support. This bit is set to 1 when the GMII Interface
> > > option is selected."
> > > 
> > > So yes, it’s likely that Serge was working with a SERDES interface which
> > > doesn't support 10/100Mbps speeds. Do you think it would be appropriate
> > > to add a check for this bit before enabling 10/100Mbps speeds?
> > 
> > DW XGMAC IP-core of v2.x and older don't support 10/100Mbps modes
> > neither in the XGMII nor in the GMII interfaces. That's why I dropped
> > the 10/100Mbps link capabilities retaining 1G, 2.5G and 10G speeds
> > only (the only speeds supported for DW XGMAC 1.20a/2.11a Tx in the
> > MAC_Tx_Configuration.SS register field). Although I should have
> > dropped the MAC_5000FD too since it has been supported since v3.0
> > IP-core version. My bad.(
> > 
> > Starting from DW XGMAC v3.00a IP-core the list of the supported speeds
> > has been extended to: 10/100Mbps (MII), 1G/2.5G (GMII), 2.5G/5G/10G
> > (XGMII). Thus the more appropriate fix here should take into account
> > the IP-core version. Like this:
> > 	if (dma_cap->mbps_1000 && MAC_Version.SNPSVER >= 0x30)
> > 		dma_cap->mbps_10_100 = 1;
> >  > Then you can use the mbps_1000 and mbps_10_100 flags to set the proper
> > MAC-capabilities to hw->link.caps in the dwxgmac2_setup() method. I
> > would have added the XGMII 2.5G/5G MAC-capabilities setting up to the
> > dwxgmac2_setup() method too for the v3.x IP-cores and newer.
> > 
> 
> Agreed. Will do in the next version.
> 
> Will ensure that mbps_10_100 is set only if SNPSVER >= 0x30 and will
> also conditionally enable 2.5G/5G MAC capabilities for IP versions
> v3.00a and above.
> 
> In the stmmac_dvr_probe() the setup() callback is invoked before
> hw_cap_support is populated. Given that, do you think it is sufficient
> to add these checks into the dwxgmac2_update_caps() instead?

Arrgh, I was looking at my local repo with a refactored hwif initialization
procedure which, amongst other things, implies the HW-features detection
performed in the setup methods.(
So in case of the vanilla STMMAC driver AFAICS there are three options
here:

1. Repeat what I did in my local repo and move the HW-features
detection procedure (calling the *_get_hw_feature() functions) to the
*_setup() methods. After that change you can use the retrieved
dma_cap-data to init the MAC capabilities exactly in sync to the
detected HW-features. But that must be thoroughly thought through
since there are Sun8i and Loongson MACs which provide their own HWIF
setup() methods (by means of plat_stmmacenet_data::setup()). So the
respective *_get_hw_feature() functions might need to be called in
these methods too (at least in the Loongson MACs setup() method).

2. Define new dwxgmac3_setup() method and thus a new entry in
stmmac_hw[]. Then dwxgmac2_setup() could be left with only 1G, 2.5G
and 10G MAC-capabilities declared, meanwhile dwxgmac3_setup() would
add all the DW XGMAC v3.00a MAC-capabilities. In this case you'd need
the dwxgmac2_update_caps() method defined anyway in order to filter
out the MAC-capabilities based on the
dma_features::{mbps_1000,mbps_10_100,half_duplex} flags state.

3. As you suggest indeed declare all the possible DW XGMAC
MAC-capabilities in the dwxgmac2_setup() method and then filter them
out in dwxgmac2_update_caps() based on the
dma_features::{mbps_1000,mbps_10_100,half_duplex} flags state and
IP-core version.

The later option seems the least code-invasive but implements a more
complex logic with declaring all the possible MAC-capabilities and
then filtering them out. Option two still implies filtering the
MAC-capabilities out but only from those specific for the particular
IP-core version. Finally option one is more complex to implement
implying the HWIF procedure refactoring with higher risks to break
things, but after it's done the setup() methods will turn to a more
useful procedures which could be used not only for the more exact
MAC-capabilities initialization but also for other
data/fields/callbacks setting up.

It's up to you and the maintainers to decide which solution would be
more appropriate.

-Serge(y)

> 
> > -Serge(y)
> > 
> > > 
> > > Best Regards,
> > > Rohan
> > > 
> 
> Best Regards,
> Rohan
> 

Re: [PATCH net-next 2/3] net: stmmac: xgmac: Correct supported speed modes

[G Thomas, Rohan]

Hi Serge,

Thanks for the detailed response.

On 7/17/2025 10:52 PM, Serge Semin wrote:
>>> DW XGMAC IP-core of v2.x and older don't support 10/100Mbps modes
>>> neither in the XGMII nor in the GMII interfaces. That's why I dropped
>>> the 10/100Mbps link capabilities retaining 1G, 2.5G and 10G speeds
>>> only (the only speeds supported for DW XGMAC 1.20a/2.11a Tx in the
>>> MAC_Tx_Configuration.SS register field). Although I should have
>>> dropped the MAC_5000FD too since it has been supported since v3.0
>>> IP-core version. My bad.(
>>>
>>> Starting from DW XGMAC v3.00a IP-core the list of the supported speeds
>>> has been extended to: 10/100Mbps (MII), 1G/2.5G (GMII), 2.5G/5G/10G
>>> (XGMII). Thus the more appropriate fix here should take into account
>>> the IP-core version. Like this:
>>> 	if (dma_cap->mbps_1000 && MAC_Version.SNPSVER >= 0x30)
>>> 		dma_cap->mbps_10_100 = 1;
>>>   > Then you can use the mbps_1000 and mbps_10_100 flags to set the proper
>>> MAC-capabilities to hw->link.caps in the dwxgmac2_setup() method. I
>>> would have added the XGMII 2.5G/5G MAC-capabilities setting up to the
>>> dwxgmac2_setup() method too for the v3.x IP-cores and newer.
>>>
>>
>> Agreed. Will do in the next version.
>>
>> Will ensure that mbps_10_100 is set only if SNPSVER >= 0x30 and will
>> also conditionally enable 2.5G/5G MAC capabilities for IP versions
>> v3.00a and above.
>>
>> In the stmmac_dvr_probe() the setup() callback is invoked before
>> hw_cap_support is populated. Given that, do you think it is sufficient
>> to add these checks into the dwxgmac2_update_caps() instead?
> 
> Arrgh, I was looking at my local repo with a refactored hwif initialization
> procedure which, amongst other things, implies the HW-features detection
> performed in the setup methods.(
> So in case of the vanilla STMMAC driver AFAICS there are three options
> here:
> 
> 1. Repeat what I did in my local repo and move the HW-features
> detection procedure (calling the *_get_hw_feature() functions) to the
> *_setup() methods. After that change you can use the retrieved
> dma_cap-data to init the MAC capabilities exactly in sync to the
> detected HW-features. But that must be thoroughly thought through
> since there are Sun8i and Loongson MACs which provide their own HWIF
> setup() methods (by means of plat_stmmacenet_data::setup()). So the
> respective *_get_hw_feature() functions might need to be called in
> these methods too (at least in the Loongson MACs setup() method).
> 
> 2. Define new dwxgmac3_setup() method and thus a new entry in
> stmmac_hw[]. Then dwxgmac2_setup() could be left with only 1G, 2.5G
> and 10G MAC-capabilities declared, meanwhile dwxgmac3_setup() would
> add all the DW XGMAC v3.00a MAC-capabilities. In this case you'd need
> the dwxgmac2_update_caps() method defined anyway in order to filter
> out the MAC-capabilities based on the
> dma_features::{mbps_1000,mbps_10_100,half_duplex} flags state.
> 
> 3. As you suggest indeed declare all the possible DW XGMAC
> MAC-capabilities in the dwxgmac2_setup() method and then filter them
> out in dwxgmac2_update_caps() based on the
> dma_features::{mbps_1000,mbps_10_100,half_duplex} flags state and
> IP-core version.
> 
> The later option seems the least code-invasive but implements a more
> complex logic with declaring all the possible MAC-capabilities and
> then filtering them out. Option two still implies filtering the
> MAC-capabilities out but only from those specific for the particular
> IP-core version. Finally option one is more complex to implement
> implying the HWIF procedure refactoring with higher risks to break
> things, but after it's done the setup() methods will turn to a more
> useful procedures which could be used not only for the more exact
> MAC-capabilities initialization but also for other
> data/fields/callbacks setting up.
> 
> It's up to you and the maintainers to decide which solution would be
> more appropriate.
> 
> -Serge(y)
> 

Unless there are concerns, I'll proceed with option 3 for now, as it’s
the least invasive and aligns well with the current driver structure.
I’ll declare all possible DW XGMAC MAC-capabilities in dwxgmac2_setup()
and then filter them appropriately in dwxgmac2_update_caps() based on
the dma_features flags and IP-core version.

Let me know if any concerns with this approach.

Best Regards,
Rohan

Re: [PATCH net-next 3/3] net: stmmac: Set CIC bit only for TX queues with COE

[Simon Horman]

On Thu, Jul 17, 2025 at 11:50:06AM +0530, G Thomas, Rohan wrote:
> Hi Simon,
> 
> On 7/16/2025 1:52 PM, Simon Horman wrote:
> > On Tue, Jul 15, 2025 at 07:14:21PM +0530, G Thomas, Rohan wrote:
> > > Hi Simon,
> > > 
> > > Thanks for reviewing the patch.
> > > 
> > > On 7/14/2025 7:10 PM, Simon Horman wrote:
> > > > On Mon, Jul 14, 2025 at 03:59:19PM +0800, Rohan G Thomas via B4 Relay wrote:
> > > > > From: Rohan G Thomas <rohan.g.thomas@altera.com>
> > > > > 
> > > > > Currently, in the AF_XDP transmit paths, the CIC bit of
> > > > > TX Desc3 is set for all packets. Setting this bit for
> > > > > packets transmitting through queues that don't support
> > > > > checksum offloading causes the TX DMA to get stuck after
> > > > > transmitting some packets. This patch ensures the CIC bit
> > > > > of TX Desc3 is set only if the TX queue supports checksum
> > > > > offloading.
> > > > > 
> > > > > Signed-off-by: Rohan G Thomas <rohan.g.thomas@altera.com>
> > > > > Reviewed-by: Matthew Gerlach <matthew.gerlach@altera.com>
> > > > 
> > > > Hi Rohan,
> > > > 
> > > > I notice that stmmac_xmit() handles a few other cases where
> > > > checksum offload should not be requested via stmmac_prepare_tx_desc:
> > > > 
> > > >           csum_insertion = (skb->ip_summed == CHECKSUM_PARTIAL);
> > > >           /* DWMAC IPs can be synthesized to support tx coe only for a few tx
> > > >            * queues. In that case, checksum offloading for those queues that don't
> > > >            * support tx coe needs to fallback to software checksum calculation.
> > > >            *
> > > >            * Packets that won't trigger the COE e.g. most DSA-tagged packets will
> > > >            * also have to be checksummed in software.
> > > >            */
> > > >           if (csum_insertion &&
> > > >               (priv->plat->tx_queues_cfg[queue].coe_unsupported ||
> > > >                !stmmac_has_ip_ethertype(skb))) {
> > > >                   if (unlikely(skb_checksum_help(skb)))
> > > >                           goto dma_map_err;
> > > >                   csum_insertion = !csum_insertion;
> > > >           }
> > > > 
> > > > Do we need to care about them in stmmac_xdp_xmit_zc()
> > > > and stmmac_xdp_xmit_xdpf() too?
> > > 
> > > This patch only addresses avoiding the TX DMA hang by ensuring the CIC
> > > bit is only set when the queue supports checksum offload. For DSA tagged
> > > packets checksum offloading is not supported by the DWMAC IPs but no TX
> > > DMA hang. AFAIK, currently AF_XDP paths don't have equivalent handling
> > > like skb_checksum_help(), since they operate on xdp buffers. So this
> > > patch doesn't attempt to implement a sw fallback but just avoids DMA
> > > stall.
> > 
> > Ok, fair enough.
> > 
> > As per Andrew's advice elsewhere in this thread.
> > This patch also looks like it should be a fix for net,
> > and should have a Fixes tag.
> 
> Thanks for your comments.
> 
> You're right—this patch is a fix for the TX DMA hang issue caused by
> setting the CIC bit on queues that don't support checksum offload. But
> I couldn’t pinpoint a specific commit that introduced this behavior in
> the AF_XDP path. Initially, there was no support for DWMAC IPs with COE
> enabled only on specific queues, even though there can be IPs with such
> configuration. Commit 8452a05b2c63 ("net: stmmac: Tx coe sw fallback")
> added software fallback support for the AF_PACKET path. But the AF_XDP
> path has always enabled COE unconditionally even before that. So, do you
> think referencing the commit 8452a05b2c63 in the Fixes tag is
> appropriate and sufficient?

Hi Rohan,

Perhaps I'm missing the point, but my thinking is as follows:

As this patch only addresses the AF_XDP path I think we can take the
approach of asking "in which patch would a user of AF_XDP with this stmmac
observe this bug". (Or some variant thereof.) And I think the answer to
that question is the patch that added AF_XDP support to stmmac driver.

So I think we can use:

Fixes: 132c32ee5bc0 ("net: stmmac: Add TX via XDP zero-copy socket")

Re: [PATCH net-next 3/3] net: stmmac: Set CIC bit only for TX queues with COE

[G Thomas, Rohan]

On 7/19/2025 1:36 AM, Simon Horman wrote:
> 
> Hi Rohan,
> 
> Perhaps I'm missing the point, but my thinking is as follows:
> 
> As this patch only addresses the AF_XDP path I think we can take the
> approach of asking "in which patch would a user of AF_XDP with this stmmac
> observe this bug". (Or some variant thereof.) And I think the answer to
> that question is the patch that added AF_XDP support to stmmac driver.
> 
> So I think we can use:
> 
> Fixes: 132c32ee5bc0 ("net: stmmac: Add TX via XDP zero-copy socket")
Hi Simon,

Thanks for the clarification. Will include the Fixes tag in the next
version.

Best Regards,
Rohan

Re: [PATCH net-next 2/3] net: stmmac: xgmac: Correct supported speed modes

[G Thomas, Rohan]

On 7/17/2025 5:17 PM, Serge Semin wrote:
> DW XGMAC IP-core of v2.x and older don't support 10/100Mbps modes
> neither in the XGMII nor in the GMII interfaces. That's why I dropped
> the 10/100Mbps link capabilities retaining 1G, 2.5G and 10G speeds
> only (the only speeds supported for DW XGMAC 1.20a/2.11a Tx in the
> MAC_Tx_Configuration.SS register field). Although I should have
> dropped the MAC_5000FD too since it has been supported since v3.0
> IP-core version. My bad.(
> 
> Starting from DW XGMAC v3.00a IP-core the list of the supported speeds
> has been extended to: 10/100Mbps (MII), 1G/2.5G (GMII), 2.5G/5G/10G
> (XGMII). Thus the more appropriate fix here should take into account
> the IP-core version. Like this:
> 	if (dma_cap->mbps_1000 && MAC_Version.SNPSVER >= 0x30)
> 		dma_cap->mbps_10_100 = 1;
> 
> Then you can use the mbps_1000 and mbps_10_100 flags to set the proper
> MAC-capabilities to hw->link.caps in the dwxgmac2_setup() method. I
> would have added the XGMII 2.5G/5G MAC-capabilities setting up to the
> dwxgmac2_setup() method too for the v3.x IP-cores and newer.

Hi Serge,

 From the databook, I noticed the condition:
(DWCXG_XGMII_GMII == 1) && <DWC-XGMAC-V2_20 feature authorized>
which seems to suggest that 10/100 Mbps support was introduced starting
from version 2.20.

Am I interpreting this correctly, or is this feature only fully
supported from v3.00 onwards.

Best Regards,
Rohan

Re: [PATCH net-next 2/3] net: stmmac: xgmac: Correct supported speed modes

[Serge Semin]

Hi Rohan

On Thu, Jul 24, 2025 at 09:48:29PM +0530, G Thomas, Rohan wrote:
> On 7/17/2025 5:17 PM, Serge Semin wrote:
> > DW XGMAC IP-core of v2.x and older don't support 10/100Mbps modes
> > neither in the XGMII nor in the GMII interfaces. That's why I dropped
> > the 10/100Mbps link capabilities retaining 1G, 2.5G and 10G speeds
> > only (the only speeds supported for DW XGMAC 1.20a/2.11a Tx in the
> > MAC_Tx_Configuration.SS register field). Although I should have
> > dropped the MAC_5000FD too since it has been supported since v3.0
> > IP-core version. My bad.(
> > 
> > Starting from DW XGMAC v3.00a IP-core the list of the supported speeds
> > has been extended to: 10/100Mbps (MII), 1G/2.5G (GMII), 2.5G/5G/10G
> > (XGMII). Thus the more appropriate fix here should take into account
> > the IP-core version. Like this:
> > 	if (dma_cap->mbps_1000 && MAC_Version.SNPSVER >= 0x30)
> > 		dma_cap->mbps_10_100 = 1;
> > 
> > Then you can use the mbps_1000 and mbps_10_100 flags to set the proper
> > MAC-capabilities to hw->link.caps in the dwxgmac2_setup() method. I
> > would have added the XGMII 2.5G/5G MAC-capabilities setting up to the
> > dwxgmac2_setup() method too for the v3.x IP-cores and newer.
> 
> Hi Serge,
> 
> From the databook, I noticed the condition:
> (DWCXG_XGMII_GMII == 1) && <DWC-XGMAC-V2_20 feature authorized>
> which seems to suggest that 10/100 Mbps support was introduced starting
> from version 2.20.
> 
> Am I interpreting this correctly, or is this feature only fully
> supported from v3.00 onwards.

Hm. Interesting discovery. I've got only DW XGMAC v1.20a, v2.11a,
v3.01a and v3.20a databooks at hands. So I can't prove for sure your
inference. But the DW XGMAC v3.01a doc indeed states that
DWCXG_FDUPLX_ONLY parameter is enabled if:
(DWCXG_XGMII_GMII == 1) && <DWC-XGMAC-V2_20 feature authorized>
which implies that the parameter was originally introduced in v2.20a
IP-core.

So to speak you might be right. Perhaps it will be more correct to set
dma_cap->mbps_10_100 for v2.20a (SNPSVER = 0x22) IP-cores too
especially seeing another parameter DWCXG_MII_SUPPORT depends on
SNPS_RSVDPARAM_16 which also depends on DWC-XGMAC-V2_20.

-Serge(y)

> 
> Best Regards,
> Rohan

Re: [PATCH net-next 2/3] net: stmmac: xgmac: Correct supported speed modes

[G Thomas, Rohan]

On 7/24/2025 11:26 PM, Serge Semin wrote:
>>> DW XGMAC IP-core of v2.x and older don't support 10/100Mbps modes
>>> neither in the XGMII nor in the GMII interfaces. That's why I dropped
>>> the 10/100Mbps link capabilities retaining 1G, 2.5G and 10G speeds
>>> only (the only speeds supported for DW XGMAC 1.20a/2.11a Tx in the
>>> MAC_Tx_Configuration.SS register field). Although I should have
>>> dropped the MAC_5000FD too since it has been supported since v3.0
>>> IP-core version. My bad.(
>>>
>>> Starting from DW XGMAC v3.00a IP-core the list of the supported speeds
>>> has been extended to: 10/100Mbps (MII), 1G/2.5G (GMII), 2.5G/5G/10G
>>> (XGMII). Thus the more appropriate fix here should take into account
>>> the IP-core version. Like this:
>>> 	if (dma_cap->mbps_1000 && MAC_Version.SNPSVER >= 0x30)
>>> 		dma_cap->mbps_10_100 = 1;
>>>
>>> Then you can use the mbps_1000 and mbps_10_100 flags to set the proper
>>> MAC-capabilities to hw->link.caps in the dwxgmac2_setup() method. I
>>> would have added the XGMII 2.5G/5G MAC-capabilities setting up to the
>>> dwxgmac2_setup() method too for the v3.x IP-cores and newer.
>>
Hi Serge,

Apologies for the multiple emails. I wanted to check specifically on the
support for 2.5G/5G over XGMII and GMII interfaces. I’ve reviewed the
v3.10a databook, but it doesn’t mention when support for these speeds
was first introduced. Could you please confirm on this?

Best Regards,
Rohan