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.1On 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!
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
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
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
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
>
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
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
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
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
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
>
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
> 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
© 2016 - 2025 Red Hat, Inc.