[PATCH net-next V2 0/2] Add support for 2500Base-X only configuration

[PATCH net-next V2 0/2] Add support for 2500Base-X only configuration

[Suraj Gupta]

Add support for 2500Base-X only configuration, which is a
synthesis option of AXI 1G/2.5G IP.

Changes in V2:
- Read 2.5G ability from Temac ability register and remove
max-speed DT property.
- Mentioned all 3 IP configurations in comment and commit description.
- Mention this series is for 2.5G only configuration.

Suraj Gupta (2):
  dt-bindings: net: xlnx,axi-ethernet: Modify descriptions and phy-mode
    value to support 2500base-X only configuration
  net: axienet: Add support for 2500base-X only configuration.

 .../bindings/net/xlnx,axi-ethernet.yaml       |  9 ++++---
 drivers/net/ethernet/xilinx/xilinx_axienet.h  |  2 +-
 .../net/ethernet/xilinx/xilinx_axienet_main.c | 24 +++++++++++++++----
 3 files changed, 26 insertions(+), 9 deletions(-)

-- 
2.25.1

[PATCH net-next V2 1/2] dt-bindings: net: xlnx,axi-ethernet: Modify descriptions and phy-mode value to support 2500base-X only configuration

[Suraj Gupta]

AXI 1G/2.5G Ethernet subsystem supports 1G and 2.5G speeds. Modify
existing binding description, pcs-handle description and add
2500base-x in phy-mode for 2500base-X only configuration.

Signed-off-by: Suraj Gupta <suraj.gupta2@amd.com>
---
 .../devicetree/bindings/net/xlnx,axi-ethernet.yaml       | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/xlnx,axi-ethernet.yaml b/Documentation/devicetree/bindings/net/xlnx,axi-ethernet.yaml
index fb02e579463c..977f55b98f31 100644
--- a/Documentation/devicetree/bindings/net/xlnx,axi-ethernet.yaml
+++ b/Documentation/devicetree/bindings/net/xlnx,axi-ethernet.yaml
@@ -9,10 +9,12 @@ title: AXI 1G/2.5G Ethernet Subsystem
 description: |
   Also called  AXI 1G/2.5G Ethernet Subsystem, the xilinx axi ethernet IP core
   provides connectivity to an external ethernet PHY supporting different
-  interfaces: MII, GMII, RGMII, SGMII, 1000BaseX. It also includes two
+  interfaces: MII, GMII, RGMII, SGMII, 1000BaseX and 2500BaseX. It also includes two
   segments of memory for buffering TX and RX, as well as the capability of
   offloading TX/RX checksum calculation off the processor.
 
+  AXI 2.5G MAC is incremental speed upgrade of AXI 1G and supports 2.5G speed.
+
   Management configuration is done through the AXI interface, while payload is
   sent and received through means of an AXI DMA controller. This driver
   includes the DMA driver code, so this driver is incompatible with AXI DMA
@@ -62,6 +64,7 @@ properties:
       - rgmii
       - sgmii
       - 1000base-x
+      - 2500base-x
 
   xlnx,phy-type:
     description:
@@ -118,8 +121,8 @@ properties:
     type: object
 
   pcs-handle:
-    description: Phandle to the internal PCS/PMA PHY in SGMII or 1000Base-X
-      modes, where "pcs-handle" should be used to point to the PCS/PMA PHY,
+    description: Phandle to the internal PCS/PMA PHY in SGMII or 1000base-x/
+      2500base-x modes, where "pcs-handle" should be used to point to the PCS/PMA PHY,
       and "phy-handle" should point to an external PHY if exists.
     maxItems: 1
 
-- 
2.25.1

Re: [PATCH net-next V2 1/2] dt-bindings: net: xlnx,axi-ethernet: Modify descriptions and phy-mode value to support 2500base-X only configuration

[Rob Herring]

On Wed, Mar 12, 2025 at 03:24:10PM +0530, Suraj Gupta wrote:
> AXI 1G/2.5G Ethernet subsystem supports 1G and 2.5G speeds. Modify
> existing binding description, pcs-handle description and add
> 2500base-x in phy-mode for 2500base-X only configuration.
> 
> Signed-off-by: Suraj Gupta <suraj.gupta2@amd.com>
> ---
>  .../devicetree/bindings/net/xlnx,axi-ethernet.yaml       | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/xlnx,axi-ethernet.yaml b/Documentation/devicetree/bindings/net/xlnx,axi-ethernet.yaml
> index fb02e579463c..977f55b98f31 100644
> --- a/Documentation/devicetree/bindings/net/xlnx,axi-ethernet.yaml
> +++ b/Documentation/devicetree/bindings/net/xlnx,axi-ethernet.yaml
> @@ -9,10 +9,12 @@ title: AXI 1G/2.5G Ethernet Subsystem
>  description: |
>    Also called  AXI 1G/2.5G Ethernet Subsystem, the xilinx axi ethernet IP core
>    provides connectivity to an external ethernet PHY supporting different
> -  interfaces: MII, GMII, RGMII, SGMII, 1000BaseX. It also includes two
> +  interfaces: MII, GMII, RGMII, SGMII, 1000BaseX and 2500BaseX. It also includes two

Please re-wrap at 80.

>    segments of memory for buffering TX and RX, as well as the capability of
>    offloading TX/RX checksum calculation off the processor.
>  
> +  AXI 2.5G MAC is incremental speed upgrade of AXI 1G and supports 2.5G speed.
> +
>    Management configuration is done through the AXI interface, while payload is
>    sent and received through means of an AXI DMA controller. This driver
>    includes the DMA driver code, so this driver is incompatible with AXI DMA
> @@ -62,6 +64,7 @@ properties:
>        - rgmii
>        - sgmii
>        - 1000base-x
> +      - 2500base-x
>  
>    xlnx,phy-type:
>      description:
> @@ -118,8 +121,8 @@ properties:
>      type: object
>  
>    pcs-handle:
> -    description: Phandle to the internal PCS/PMA PHY in SGMII or 1000Base-X
> -      modes, where "pcs-handle" should be used to point to the PCS/PMA PHY,
> +    description: Phandle to the internal PCS/PMA PHY in SGMII or 1000base-x/
> +      2500base-x modes, where "pcs-handle" should be used to point to the PCS/PMA PHY,

And here.

>        and "phy-handle" should point to an external PHY if exists.
>      maxItems: 1
>  
> -- 
> 2.25.1
> 

[PATCH net-next V2 2/2] net: axienet: Add support for 2500base-X only configuration.

[Suraj Gupta]

AXI 1G/2.5G ethernet IP has following synthesis options:
1) SGMII/1000base-X only.
2) 2500base-X only.
3) dynamically switching between (1) and (2).
Add support for 2500base-X only configuration.

Co-developed-by: Harini Katakam <harini.katakam@amd.com>
Signed-off-by: Harini Katakam <harini.katakam@amd.com>
Signed-off-by: Suraj Gupta <suraj.gupta2@amd.com>
---
 drivers/net/ethernet/xilinx/xilinx_axienet.h  |  2 +-
 .../net/ethernet/xilinx/xilinx_axienet_main.c | 24 +++++++++++++++----
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet.h b/drivers/net/ethernet/xilinx/xilinx_axienet.h
index 5ff742103beb..ded3e32999d5 100644
--- a/drivers/net/ethernet/xilinx/xilinx_axienet.h
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet.h
@@ -274,7 +274,7 @@
 #define XAE_EMMC_RX16BIT	0x01000000 /* 16 bit Rx client enable */
 #define XAE_EMMC_LINKSPD_10	0x00000000 /* Link Speed mask for 10 Mbit */
 #define XAE_EMMC_LINKSPD_100	0x40000000 /* Link Speed mask for 100 Mbit */
-#define XAE_EMMC_LINKSPD_1000	0x80000000 /* Link Speed mask for 1000 Mbit */
+#define XAE_EMMC_LINKSPD_1000_2500	0x80000000 /* Link Speed mask for 1000 or 2500 Mbit */
 
 /* Bit masks for Axi Ethernet PHYC register */
 #define XAE_PHYC_SGMIILINKSPEED_MASK	0xC0000000 /* SGMII link speed mask*/
diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
index 054abf283ab3..0885ce201b0a 100644
--- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
@@ -2583,6 +2583,7 @@ static struct phylink_pcs *axienet_mac_select_pcs(struct phylink_config *config,
 	struct axienet_local *lp = netdev_priv(ndev);
 
 	if (interface == PHY_INTERFACE_MODE_1000BASEX ||
+	    interface == PHY_INTERFACE_MODE_2500BASEX ||
 	    interface ==  PHY_INTERFACE_MODE_SGMII)
 		return &lp->pcs;
 
@@ -2616,8 +2617,9 @@ static void axienet_mac_link_up(struct phylink_config *config,
 	emmc_reg &= ~XAE_EMMC_LINKSPEED_MASK;
 
 	switch (speed) {
+	case SPEED_2500:
 	case SPEED_1000:
-		emmc_reg |= XAE_EMMC_LINKSPD_1000;
+		emmc_reg |= XAE_EMMC_LINKSPD_1000_2500;
 		break;
 	case SPEED_100:
 		emmc_reg |= XAE_EMMC_LINKSPD_100;
@@ -2627,7 +2629,7 @@ static void axienet_mac_link_up(struct phylink_config *config,
 		break;
 	default:
 		dev_err(&ndev->dev,
-			"Speed other than 10, 100 or 1Gbps is not supported\n");
+			"Speed other than 10, 100, 1Gbps or 2.5Gbps is not supported\n");
 		break;
 	}
 
@@ -3055,7 +3057,8 @@ static int axienet_probe(struct platform_device *pdev)
 			 "error registering MDIO bus: %d\n", ret);
 
 	if (lp->phy_mode == PHY_INTERFACE_MODE_SGMII ||
-	    lp->phy_mode == PHY_INTERFACE_MODE_1000BASEX) {
+	    lp->phy_mode == PHY_INTERFACE_MODE_1000BASEX ||
+	    lp->phy_mode == PHY_INTERFACE_MODE_2500BASEX) {
 		np = of_parse_phandle(pdev->dev.of_node, "pcs-handle", 0);
 		if (!np) {
 			/* Deprecated: Always use "pcs-handle" for pcs_phy.
@@ -3083,8 +3086,19 @@ static int axienet_probe(struct platform_device *pdev)
 	lp->phylink_config.dev = &ndev->dev;
 	lp->phylink_config.type = PHYLINK_NETDEV;
 	lp->phylink_config.mac_managed_pm = true;
-	lp->phylink_config.mac_capabilities = MAC_SYM_PAUSE | MAC_ASYM_PAUSE |
-		MAC_10FD | MAC_100FD | MAC_1000FD;
+	lp->phylink_config.mac_capabilities = MAC_SYM_PAUSE | MAC_ASYM_PAUSE;
+
+	/* AXI 1G/2.5G ethernet IP has following synthesis options:
+	 * 1) SGMII/1000base-X only.
+	 * 2) 2500base-X only.
+	 * 3) Dynamically switching between (1) and (2), and is not
+	 * implemented in driver.
+	 */
+
+	if (axienet_ior(lp, XAE_ABILITY_OFFSET) & XAE_ABILITY_2_5G)
+		lp->phylink_config.mac_capabilities |= MAC_2500FD;
+	else
+		lp->phylink_config.mac_capabilities |= MAC_10FD | MAC_100FD | MAC_1000FD;
 
 	__set_bit(lp->phy_mode, lp->phylink_config.supported_interfaces);
 	if (lp->switch_x_sgmii) {
-- 
2.25.1

Re: [PATCH net-next V2 2/2] net: axienet: Add support for 2500base-X only configuration.

[Dawid Osuchowski]

On 2025-03-12 10:54 AM, Suraj Gupta wrote:
> AXI 1G/2.5G ethernet IP has following synthesis options:
> 1) SGMII/1000base-X only.
> 2) 2500base-X only.
> 3) dynamically switching between (1) and (2).
> Add support for 2500base-X only configuration.

Hi, thanks for the patch.

nit: a discrepancy between the commit description for and the comments 
in the code for 3)

Maybe adding that information here in the commit description would make 
sense as well? Or giving a bit of a background that SGMII/1000base-X is 
already implemented in the driver and you are adding 2500base-X only 
support.

> +	/* AXI 1G/2.5G ethernet IP has following synthesis options:
> +	 * 1) SGMII/1000base-X only.
> +	 * 2) 2500base-X only.
> +	 * 3) Dynamically switching between (1) and (2), and is not
> +	 * implemented in driver.
> +	 */

For the rest of the patch, it looks good to me but I'd rather have 
someone more experienced provide the Reviewed-By tag if they find the 
patch appropriate.

Best regards,
Dawid

Re: [PATCH net-next V2 2/2] net: axienet: Add support for 2500base-X only configuration.

[Andrew Lunn]

> +	/* AXI 1G/2.5G ethernet IP has following synthesis options:
> +	 * 1) SGMII/1000base-X only.
> +	 * 2) 2500base-X only.
> +	 * 3) Dynamically switching between (1) and (2), and is not
> +	 * implemented in driver.
> +	 */
> +
> +	if (axienet_ior(lp, XAE_ABILITY_OFFSET) & XAE_ABILITY_2_5G)

How can we tell if the synthesis allows 3)?

Don't we have a backwards compatibility issue here? Maybe there are
systems which have been synthesised with 3), but are currently limited
to 1) due to the driver. If you don't differentiate between 2 and 3,
such systems are going to swap to 2) and regress.

	Andrew

Re: [PATCH net-next V2 2/2] net: axienet: Add support for 2500base-X only configuration.

[Russell King (Oracle)]

On Wed, Mar 12, 2025 at 02:25:27PM +0100, Andrew Lunn wrote:
> > +	/* AXI 1G/2.5G ethernet IP has following synthesis options:
> > +	 * 1) SGMII/1000base-X only.
> > +	 * 2) 2500base-X only.
> > +	 * 3) Dynamically switching between (1) and (2), and is not
> > +	 * implemented in driver.
> > +	 */
> > +
> > +	if (axienet_ior(lp, XAE_ABILITY_OFFSET) & XAE_ABILITY_2_5G)
> 
> How can we tell if the synthesis allows 3)?
> 
> Don't we have a backwards compatibility issue here? Maybe there are
> systems which have been synthesised with 3), but are currently limited
> to 1) due to the driver. If you don't differentiate between 2 and 3,
> such systems are going to swap to 2) and regress.

We've discussed this before... but because the author doesn't post
regularly enough, it's not suprising that context keeps getting lost.

Here's the discussion from 20th February 2025 on a patch series that I
commented on on 19th November 2024.

https://lore.kernel.org/r/BL3PR12MB6571FE73FA8D5AAB9FB4BB3CC9C42@BL3PR12MB6571.namprd12.prod.outlook.com

Suraj Gupta - you _must_ be more responsive so that reviewers can keep
the context of previous discussions in their heads to avoid going over
the same points time and time again. If you can't do that (and it's a
good idea anyway) then you need to supplement the commit descriptions
with the salient points from the previous patch series discussion to
remind reviewers of the appropriate context.

-- 
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 V2 2/2] net: axienet: Add support for 2500base-X only configuration.

[Gupta, Suraj]

[AMD Official Use Only - AMD Internal Distribution Only]

> -----Original Message-----
> From: Russell King <linux@armlinux.org.uk>
> Sent: Wednesday, March 12, 2025 7:44 PM
> To: Andrew Lunn <andrew@lunn.ch>
> Cc: Gupta, Suraj <Suraj.Gupta2@amd.com>; Pandey, Radhey Shyam
> <radhey.shyam.pandey@amd.com>; andrew+netdev@lunn.ch;
> davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> pabeni@redhat.com; robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org;
> Simek, Michal <michal.simek@amd.com>; netdev@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; git (AMD-Xilinx) <git@amd.com>; Katakam, Harini
> <harini.katakam@amd.com>
> Subject: Re: [PATCH net-next V2 2/2] net: axienet: Add support for 2500base-X only
> configuration.
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> On Wed, Mar 12, 2025 at 02:25:27PM +0100, Andrew Lunn wrote:
> > > +   /* AXI 1G/2.5G ethernet IP has following synthesis options:
> > > +    * 1) SGMII/1000base-X only.
> > > +    * 2) 2500base-X only.
> > > +    * 3) Dynamically switching between (1) and (2), and is not
> > > +    * implemented in driver.
> > > +    */
> > > +
> > > +   if (axienet_ior(lp, XAE_ABILITY_OFFSET) & XAE_ABILITY_2_5G)
> >
> > How can we tell if the synthesis allows 3)?
> >
> > Don't we have a backwards compatibility issue here? Maybe there are
> > systems which have been synthesised with 3), but are currently limited
> > to 1) due to the driver. If you don't differentiate between 2 and 3,
> > such systems are going to swap to 2) and regress.
>
> We've discussed this before... but because the author doesn't post regularly enough,
> it's not suprising that context keeps getting lost.
>
> Here's the discussion from 20th February 2025 on a patch series that I commented
> on on 19th November 2024.
>
> https://lore.kernel.org/r/BL3PR12MB6571FE73FA8D5AAB9FB4BB3CC9C42@BL3
> PR12MB6571.namprd12.prod.outlook.com
>
> Suraj Gupta - you _must_ be more responsive so that reviewers can keep the
> context of previous discussions in their heads to avoid going over the same points
> time and time again. If you can't do that (and it's a good idea anyway) then you need
> to supplement the commit descriptions with the salient points from the previous
> patch series discussion to remind reviewers of the appropriate context.
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Thanks Russell, sure I'll add salient points from previous discussions if it becomes old (won't try to delay at first).
Andrew - Keeping previous discussion short, identification of (3) depends on how user implements switching logic in FPGA (external GT or RTL logic). AXI 1G/2.5G IP provides only static speed selections and there is no standard register to communicate that to software.

Re: [PATCH net-next V2 2/2] net: axienet: Add support for 2500base-X only configuration.

[Andrew Lunn]

> > On Wed, Mar 12, 2025 at 02:25:27PM +0100, Andrew Lunn wrote:
> > > > +   /* AXI 1G/2.5G ethernet IP has following synthesis options:
> > > > +    * 1) SGMII/1000base-X only.
> > > > +    * 2) 2500base-X only.
> > > > +    * 3) Dynamically switching between (1) and (2), and is not
> > > > +    * implemented in driver.
> > > > +    */

> - Keeping previous discussion short, identification of (3) depends
> on how user implements switching logic in FPGA (external GT or RTL
> logic). AXI 1G/2.5G IP provides only static speed selections and
> there is no standard register to communicate that to software.

So if anybody has synthesised it as 3) this change will break their
system?

	Andrew

RE: [PATCH net-next V2 2/2] net: axienet: Add support for 2500base-X only configuration.

[Gupta, Suraj]

[AMD Official Use Only - AMD Internal Distribution Only]

> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: Wednesday, March 12, 2025 8:29 PM
> To: Gupta, Suraj <Suraj.Gupta2@amd.com>
> Cc: Russell King <linux@armlinux.org.uk>; Pandey, Radhey Shyam
> <radhey.shyam.pandey@amd.com>; andrew+netdev@lunn.ch;
> davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> pabeni@redhat.com; robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org;
> Simek, Michal <michal.simek@amd.com>; netdev@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; git (AMD-Xilinx) <git@amd.com>; Katakam, Harini
> <harini.katakam@amd.com>
> Subject: Re: [PATCH net-next V2 2/2] net: axienet: Add support for 2500base-X only
> configuration.
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> > > On Wed, Mar 12, 2025 at 02:25:27PM +0100, Andrew Lunn wrote:
> > > > > +   /* AXI 1G/2.5G ethernet IP has following synthesis options:
> > > > > +    * 1) SGMII/1000base-X only.
> > > > > +    * 2) 2500base-X only.
> > > > > +    * 3) Dynamically switching between (1) and (2), and is not
> > > > > +    * implemented in driver.
> > > > > +    */
>
> > - Keeping previous discussion short, identification of (3) depends on
> > how user implements switching logic in FPGA (external GT or RTL
> > logic). AXI 1G/2.5G IP provides only static speed selections and there
> > is no standard register to communicate that to software.
>
> So if anybody has synthesised it as 3) this change will break their system?
>
>         Andrew

It will just restrict their system to (2)

Re: [PATCH net-next V2 2/2] net: axienet: Add support for 2500base-X only configuration.

[Andrew Lunn]

On Wed, Mar 12, 2025 at 03:06:32PM +0000, Gupta, Suraj wrote:
> [AMD Official Use Only - AMD Internal Distribution Only]
> 
> > -----Original Message-----
> > From: Andrew Lunn <andrew@lunn.ch>
> > Sent: Wednesday, March 12, 2025 8:29 PM
> > To: Gupta, Suraj <Suraj.Gupta2@amd.com>
> > Cc: Russell King <linux@armlinux.org.uk>; Pandey, Radhey Shyam
> > <radhey.shyam.pandey@amd.com>; andrew+netdev@lunn.ch;
> > davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> > pabeni@redhat.com; robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org;
> > Simek, Michal <michal.simek@amd.com>; netdev@vger.kernel.org;
> > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-
> > kernel@lists.infradead.org; git (AMD-Xilinx) <git@amd.com>; Katakam, Harini
> > <harini.katakam@amd.com>
> > Subject: Re: [PATCH net-next V2 2/2] net: axienet: Add support for 2500base-X only
> > configuration.
> >
> > Caution: This message originated from an External Source. Use proper caution
> > when opening attachments, clicking links, or responding.
> >
> >
> > > > On Wed, Mar 12, 2025 at 02:25:27PM +0100, Andrew Lunn wrote:
> > > > > > +   /* AXI 1G/2.5G ethernet IP has following synthesis options:
> > > > > > +    * 1) SGMII/1000base-X only.
> > > > > > +    * 2) 2500base-X only.
> > > > > > +    * 3) Dynamically switching between (1) and (2), and is not
> > > > > > +    * implemented in driver.
> > > > > > +    */
> >
> > > - Keeping previous discussion short, identification of (3) depends on
> > > how user implements switching logic in FPGA (external GT or RTL
> > > logic). AXI 1G/2.5G IP provides only static speed selections and there
> > > is no standard register to communicate that to software.
> >
> > So if anybody has synthesised it as 3) this change will break their system?
> >
> >         Andrew
> 
> It will just restrict their system to (2)

Where as before, it was doing SGMII/1000base-X only. So such systems
break?

	Andrew

RE: [PATCH net-next V2 2/2] net: axienet: Add support for 2500base-X only configuration.

[Gupta, Suraj]

[AMD Official Use Only - AMD Internal Distribution Only]

> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: Wednesday, March 12, 2025 9:03 PM
> To: Gupta, Suraj <Suraj.Gupta2@amd.com>
> Cc: Russell King <linux@armlinux.org.uk>; Pandey, Radhey Shyam
> <radhey.shyam.pandey@amd.com>; andrew+netdev@lunn.ch;
> davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> pabeni@redhat.com; robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org;
> Simek, Michal <michal.simek@amd.com>; netdev@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; git (AMD-Xilinx) <git@amd.com>; Katakam, Harini
> <harini.katakam@amd.com>
> Subject: Re: [PATCH net-next V2 2/2] net: axienet: Add support for 2500base-X only
> configuration.
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> On Wed, Mar 12, 2025 at 03:06:32PM +0000, Gupta, Suraj wrote:
> > [AMD Official Use Only - AMD Internal Distribution Only]
> >
> > > -----Original Message-----
> > > From: Andrew Lunn <andrew@lunn.ch>
> > > Sent: Wednesday, March 12, 2025 8:29 PM
> > > To: Gupta, Suraj <Suraj.Gupta2@amd.com>
> > > Cc: Russell King <linux@armlinux.org.uk>; Pandey, Radhey Shyam
> > > <radhey.shyam.pandey@amd.com>; andrew+netdev@lunn.ch;
> > > davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> > > pabeni@redhat.com; robh@kernel.org; krzk+dt@kernel.org;
> > > conor+dt@kernel.org; Simek, Michal <michal.simek@amd.com>;
> > > netdev@vger.kernel.org; devicetree@vger.kernel.org;
> > > linux-kernel@vger.kernel.org; linux-arm- kernel@lists.infradead.org;
> > > git (AMD-Xilinx) <git@amd.com>; Katakam, Harini
> > > <harini.katakam@amd.com>
> > > Subject: Re: [PATCH net-next V2 2/2] net: axienet: Add support for
> > > 2500base-X only configuration.
> > >
> > > Caution: This message originated from an External Source. Use proper
> > > caution when opening attachments, clicking links, or responding.
> > >
> > >
> > > > > On Wed, Mar 12, 2025 at 02:25:27PM +0100, Andrew Lunn wrote:
> > > > > > > +   /* AXI 1G/2.5G ethernet IP has following synthesis options:
> > > > > > > +    * 1) SGMII/1000base-X only.
> > > > > > > +    * 2) 2500base-X only.
> > > > > > > +    * 3) Dynamically switching between (1) and (2), and is not
> > > > > > > +    * implemented in driver.
> > > > > > > +    */
> > >
> > > > - Keeping previous discussion short, identification of (3) depends
> > > > on how user implements switching logic in FPGA (external GT or RTL
> > > > logic). AXI 1G/2.5G IP provides only static speed selections and
> > > > there is no standard register to communicate that to software.
> > >
> > > So if anybody has synthesised it as 3) this change will break their system?
> > >
> > >         Andrew
> >
> > It will just restrict their system to (2)
>
> Where as before, it was doing SGMII/1000base-X only. So such systems break?
>
>         Andrew

If the user wants (3), they need to add their custom FPGA logic which anyway will require additional driver changes. (3) was not completely supported by existing driver.

Re: [PATCH net-next V2 2/2] net: axienet: Add support for 2500base-X only configuration.

[Andrew Lunn]

On Wed, Mar 12, 2025 at 04:08:02PM +0000, Gupta, Suraj wrote:
> [AMD Official Use Only - AMD Internal Distribution Only]
> 
> > -----Original Message-----
> > From: Andrew Lunn <andrew@lunn.ch>
> > Sent: Wednesday, March 12, 2025 9:03 PM
> > To: Gupta, Suraj <Suraj.Gupta2@amd.com>
> > Cc: Russell King <linux@armlinux.org.uk>; Pandey, Radhey Shyam
> > <radhey.shyam.pandey@amd.com>; andrew+netdev@lunn.ch;
> > davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> > pabeni@redhat.com; robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org;
> > Simek, Michal <michal.simek@amd.com>; netdev@vger.kernel.org;
> > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-
> > kernel@lists.infradead.org; git (AMD-Xilinx) <git@amd.com>; Katakam, Harini
> > <harini.katakam@amd.com>
> > Subject: Re: [PATCH net-next V2 2/2] net: axienet: Add support for 2500base-X only
> > configuration.
> >
> > Caution: This message originated from an External Source. Use proper caution
> > when opening attachments, clicking links, or responding.
> >
> >
> > On Wed, Mar 12, 2025 at 03:06:32PM +0000, Gupta, Suraj wrote:
> > > [AMD Official Use Only - AMD Internal Distribution Only]
> > >
> > > > -----Original Message-----
> > > > From: Andrew Lunn <andrew@lunn.ch>
> > > > Sent: Wednesday, March 12, 2025 8:29 PM
> > > > To: Gupta, Suraj <Suraj.Gupta2@amd.com>
> > > > Cc: Russell King <linux@armlinux.org.uk>; Pandey, Radhey Shyam
> > > > <radhey.shyam.pandey@amd.com>; andrew+netdev@lunn.ch;
> > > > davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> > > > pabeni@redhat.com; robh@kernel.org; krzk+dt@kernel.org;
> > > > conor+dt@kernel.org; Simek, Michal <michal.simek@amd.com>;
> > > > netdev@vger.kernel.org; devicetree@vger.kernel.org;
> > > > linux-kernel@vger.kernel.org; linux-arm- kernel@lists.infradead.org;
> > > > git (AMD-Xilinx) <git@amd.com>; Katakam, Harini
> > > > <harini.katakam@amd.com>
> > > > Subject: Re: [PATCH net-next V2 2/2] net: axienet: Add support for
> > > > 2500base-X only configuration.
> > > >
> > > > Caution: This message originated from an External Source. Use proper
> > > > caution when opening attachments, clicking links, or responding.
> > > >
> > > >
> > > > > > On Wed, Mar 12, 2025 at 02:25:27PM +0100, Andrew Lunn wrote:
> > > > > > > > +   /* AXI 1G/2.5G ethernet IP has following synthesis options:
> > > > > > > > +    * 1) SGMII/1000base-X only.
> > > > > > > > +    * 2) 2500base-X only.
> > > > > > > > +    * 3) Dynamically switching between (1) and (2), and is not
> > > > > > > > +    * implemented in driver.
> > > > > > > > +    */
> > > >
> > > > > - Keeping previous discussion short, identification of (3) depends
> > > > > on how user implements switching logic in FPGA (external GT or RTL
> > > > > logic). AXI 1G/2.5G IP provides only static speed selections and
> > > > > there is no standard register to communicate that to software.
> > > >
> > > > So if anybody has synthesised it as 3) this change will break their system?
> > > >
> > > >         Andrew
> > >
> > > It will just restrict their system to (2)
> >
> > Where as before, it was doing SGMII/1000base-X only. So such systems break?
> >
> >         Andrew
> 

> If the user wants (3), they need to add their custom FPGA logic
> which anyway will require additional driver changes. (3) was not
> completely supported by existing driver.

You say 3) is a synthesis option. Say somebody synthesised it that
way, and found it works for what they need with SGMII/1000base-X.
Because the driver took no notice of the capability bit, that is what
it would do. Since it worked for them, they might not of gone back and
optimised the options. "If it is not broken, don't fix it". So we
could have systems out in the wild, synthesised as 3) happily doing 
SGMII/1000base-X ?

With this change, won't you break those systems?

I'm just trying to get to a definitive answer, is this change actually
safe to all todays possible systems?

	Andrew

Re: [PATCH net-next V2 2/2] net: axienet: Add support for 2500base-X only configuration.

[Russell King (Oracle)]

On Wed, Mar 12, 2025 at 04:08:02PM +0000, Gupta, Suraj wrote:
> [AMD Official Use Only - AMD Internal Distribution Only]
> 
> > -----Original Message-----
> > From: Andrew Lunn <andrew@lunn.ch>
> > Sent: Wednesday, March 12, 2025 9:03 PM
> > To: Gupta, Suraj <Suraj.Gupta2@amd.com>
> > Cc: Russell King <linux@armlinux.org.uk>; Pandey, Radhey Shyam
> > <radhey.shyam.pandey@amd.com>; andrew+netdev@lunn.ch;
> > davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> > pabeni@redhat.com; robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org;
> > Simek, Michal <michal.simek@amd.com>; netdev@vger.kernel.org;
> > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-
> > kernel@lists.infradead.org; git (AMD-Xilinx) <git@amd.com>; Katakam, Harini
> > <harini.katakam@amd.com>
> > Subject: Re: [PATCH net-next V2 2/2] net: axienet: Add support for 2500base-X only
> > configuration.
> >
> > Caution: This message originated from an External Source. Use proper caution
> > when opening attachments, clicking links, or responding.
> >
> >
> > On Wed, Mar 12, 2025 at 03:06:32PM +0000, Gupta, Suraj wrote:
> > > [AMD Official Use Only - AMD Internal Distribution Only]
> > >
> > > > -----Original Message-----
> > > > From: Andrew Lunn <andrew@lunn.ch>
> > > > Sent: Wednesday, March 12, 2025 8:29 PM
> > > > To: Gupta, Suraj <Suraj.Gupta2@amd.com>
> > > > Cc: Russell King <linux@armlinux.org.uk>; Pandey, Radhey Shyam
> > > > <radhey.shyam.pandey@amd.com>; andrew+netdev@lunn.ch;
> > > > davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> > > > pabeni@redhat.com; robh@kernel.org; krzk+dt@kernel.org;
> > > > conor+dt@kernel.org; Simek, Michal <michal.simek@amd.com>;
> > > > netdev@vger.kernel.org; devicetree@vger.kernel.org;
> > > > linux-kernel@vger.kernel.org; linux-arm- kernel@lists.infradead.org;
> > > > git (AMD-Xilinx) <git@amd.com>; Katakam, Harini
> > > > <harini.katakam@amd.com>
> > > > Subject: Re: [PATCH net-next V2 2/2] net: axienet: Add support for
> > > > 2500base-X only configuration.
> > > >
> > > > Caution: This message originated from an External Source. Use proper
> > > > caution when opening attachments, clicking links, or responding.
> > > >
> > > >
> > > > > > On Wed, Mar 12, 2025 at 02:25:27PM +0100, Andrew Lunn wrote:
> > > > > > > > +   /* AXI 1G/2.5G ethernet IP has following synthesis options:
> > > > > > > > +    * 1) SGMII/1000base-X only.
> > > > > > > > +    * 2) 2500base-X only.
> > > > > > > > +    * 3) Dynamically switching between (1) and (2), and is not
> > > > > > > > +    * implemented in driver.
> > > > > > > > +    */
> > > >
> > > > > - Keeping previous discussion short, identification of (3) depends
> > > > > on how user implements switching logic in FPGA (external GT or RTL
> > > > > logic). AXI 1G/2.5G IP provides only static speed selections and
> > > > > there is no standard register to communicate that to software.
> > > >
> > > > So if anybody has synthesised it as 3) this change will break their system?
> > > >
> > > >         Andrew
> > >
> > > It will just restrict their system to (2)
> >
> > Where as before, it was doing SGMII/1000base-X only. So such systems break?
> >
> >         Andrew
> 
> If the user wants (3), they need to add their custom FPGA logic which anyway will require additional driver changes. (3) was not completely supported by existing driver.

This is not an approach that works with the Linux kernel, sorry.

What we have today is a driver that works for people's hardware - and
we don't know what the capabilities of that hardware is.

If there's hardware out there today which has XAE_ABILITY_2_5G set, but
defaults to <=1G mode, this will work with the current driver. However,
with your patch applied, it stops working because instead of the driver
indicating MAC_10FD | MAC_100FD | MAC_1000FD, it only indicates
MAC_2500FD. If this happens, it will regress users setups, and that is
something we try not to do.

Saying "someone else needs to add the code for their FPGA logic" misses
the point - there may not be "someone else" to do that, which means
the only option is to revert your change if it were merged. That in
itself can cause its own user regressions because obviously stuff that
works with this patch stops working.

This is why we're being cautious, and given your responses, it's not
making Andrew or myself feel that there's a reasonable approach being
taken here.

From everything you have said, I am getting the feeling that using
XAE_ABILITY_2_5G to decide which of (1) or (2) is supported is just
wrong. Given that we're talking about an implementation that has been
synthesized at 2.5G and can't operate slower, maybe there's some way
that could be created to specify that in DT?

e.g. (and I'm sure the DT folk aren't going to like it)...

	xlnx,axi-ethernet-X.YY.Z-2.5G

(where X.YY.Z is the version) for implementations that can _only_ do
2.5G, and leave all other implementations only doing 1G and below.

Or maybe some DT property. Or something else.

-- 
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 V2 2/2] net: axienet: Add support for 2500base-X only configuration.

[Andrew Lunn]

> This is not an approach that works with the Linux kernel, sorry.
> 
> What we have today is a driver that works for people's hardware - and
> we don't know what the capabilities of that hardware is.
> 
> If there's hardware out there today which has XAE_ABILITY_2_5G set, but
> defaults to <=1G mode, this will work with the current driver. However,
> with your patch applied, it stops working because instead of the driver
> indicating MAC_10FD | MAC_100FD | MAC_1000FD, it only indicates
> MAC_2500FD. If this happens, it will regress users setups, and that is
> something we try not to do.
> 
> Saying "someone else needs to add the code for their FPGA logic" misses
> the point - there may not be "someone else" to do that, which means
> the only option is to revert your change if it were merged. That in
> itself can cause its own user regressions because obviously stuff that
> works with this patch stops working.
> 
> This is why we're being cautious, and given your responses, it's not
> making Andrew or myself feel that there's a reasonable approach being
> taken here.
> 
> >From everything you have said, I am getting the feeling that using
> XAE_ABILITY_2_5G to decide which of (1) or (2) is supported is just
> wrong. Given that we're talking about an implementation that has been
> synthesized at 2.5G and can't operate slower, maybe there's some way
> that could be created to specify that in DT?
> 
> e.g. (and I'm sure the DT folk aren't going to like it)...
> 
> 	xlnx,axi-ethernet-X.YY.Z-2.5G
> 
> (where X.YY.Z is the version) for implementations that can _only_ do
> 2.5G, and leave all other implementations only doing 1G and below.
> 
> Or maybe some DT property. Or something else.

Given that AMD has been talking about an FPGA, not silicon, i actually
think it would be best to change the IP to explicitly enumerate how it
has been synthesised. Make use of some register bits which currently
read as 0. Current IP would then remain as 1000BaseX/SGMII,
independent of how they have been synthesised. Newer versions of the
IP will then set the bits if they have been synthesised as 2) or 3),
and the driver can then enable that capability, without breaking
current generation systems. Plus there needs to be big fat warning for
anybody upgrading to the latest version of the IP for bug fixes to
ensure they correctly set the synthesis options because it now
actually matters.

	 Andrew

RE: [PATCH net-next V2 2/2] net: axienet: Add support for 2500base-X only configuration.

[Gupta, Suraj]

[AMD Official Use Only - AMD Internal Distribution Only]

> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: Thursday, March 13, 2025 3:41 AM
> To: Russell King (Oracle) <linux@armlinux.org.uk>
> Cc: Gupta, Suraj <Suraj.Gupta2@amd.com>; Pandey, Radhey Shyam
> <radhey.shyam.pandey@amd.com>; andrew+netdev@lunn.ch;
> davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> pabeni@redhat.com; robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org;
> Simek, Michal <michal.simek@amd.com>; netdev@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; git (AMD-Xilinx) <git@amd.com>; Katakam, Harini
> <harini.katakam@amd.com>
> Subject: Re: [PATCH net-next V2 2/2] net: axienet: Add support for 2500base-X only
> configuration.
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> > This is not an approach that works with the Linux kernel, sorry.
> >
> > What we have today is a driver that works for people's hardware - and
> > we don't know what the capabilities of that hardware is.
> >
> > If there's hardware out there today which has XAE_ABILITY_2_5G set,
> > but defaults to <=1G mode, this will work with the current driver.
> > However, with your patch applied, it stops working because instead of
> > the driver indicating MAC_10FD | MAC_100FD | MAC_1000FD, it only
> > indicates MAC_2500FD. If this happens, it will regress users setups,
> > and that is something we try not to do.
> >
> > Saying "someone else needs to add the code for their FPGA logic"
> > misses the point - there may not be "someone else" to do that, which
> > means the only option is to revert your change if it were merged. That
> > in itself can cause its own user regressions because obviously stuff
> > that works with this patch stops working.
> >
> > This is why we're being cautious, and given your responses, it's not
> > making Andrew or myself feel that there's a reasonable approach being
> > taken here.
> >
> > >From everything you have said, I am getting the feeling that using
> > XAE_ABILITY_2_5G to decide which of (1) or (2) is supported is just
> > wrong. Given that we're talking about an implementation that has been
> > synthesized at 2.5G and can't operate slower, maybe there's some way
> > that could be created to specify that in DT?
> >
> > e.g. (and I'm sure the DT folk aren't going to like it)...
> >
> >       xlnx,axi-ethernet-X.YY.Z-2.5G
> >
> > (where X.YY.Z is the version) for implementations that can _only_ do
> > 2.5G, and leave all other implementations only doing 1G and below.
> >
> > Or maybe some DT property. Or something else.
>
> Given that AMD has been talking about an FPGA, not silicon, i actually think it would
> be best to change the IP to explicitly enumerate how it has been synthesised. Make
> use of some register bits which currently read as 0. Current IP would then remain as
> 1000BaseX/SGMII, independent of how they have been synthesised. Newer
> versions of the IP will then set the bits if they have been synthesised as 2) or 3), and
> the driver can then enable that capability, without breaking current generation
> systems. Plus there needs to be big fat warning for anybody upgrading to the latest
> version of the IP for bug fixes to ensure they correctly set the synthesis options
> because it now actually matters.
>
>          Andrew

Synthesis options I mentioned in comment might sound confusing, let me clear it up.
Actual synthesis options (as seen from configuration UI) IP provides are (1) and (2). When a user selects (2), IP comes with default 2.5G but also contains 1G capabilities which can be enabled and work with by adding switching FPGA logic (that makes it (3)).

So, in short  if a user selects (1): It's <=1G only.
If it selects (2): It's 2.5G only but can be made (3) by FPGA logic changes. So whatever existing systems for (3) would be working at default (2).

This is the reason we didn't described (3) in V1 series as that is not provided by IP but can be synthesized after FPGA changes.
Hope I'm able to answer your questions.

RE: [PATCH net-next V2 2/2] net: axienet: Add support for 2500base-X only configuration.

[Gupta, Suraj]

[AMD Official Use Only - AMD Internal Distribution Only]

> -----Original Message-----
> From: Gupta, Suraj
> Sent: Thursday, March 13, 2025 9:01 AM
> To: Andrew Lunn <andrew@lunn.ch>; Russell King (Oracle)
> <linux@armlinux.org.uk>
> Cc: Pandey, Radhey Shyam <radhey.shyam.pandey@amd.com>;
> andrew+netdev@lunn.ch; davem@davemloft.net; edumazet@google.com;
> kuba@kernel.org; pabeni@redhat.com; robh@kernel.org; krzk+dt@kernel.org;
> conor+dt@kernel.org; Simek, Michal <michal.simek@amd.com>;
> netdev@vger.kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; git (AMD-Xilinx) <git@amd.com>; Katakam,
> Harini <harini.katakam@amd.com>
> Subject: RE: [PATCH net-next V2 2/2] net: axienet: Add support for 2500base-X only
> configuration.
>
>
>
> > -----Original Message-----
> > From: Andrew Lunn <andrew@lunn.ch>
> > Sent: Thursday, March 13, 2025 3:41 AM
> > To: Russell King (Oracle) <linux@armlinux.org.uk>
> > Cc: Gupta, Suraj <Suraj.Gupta2@amd.com>; Pandey, Radhey Shyam
> > <radhey.shyam.pandey@amd.com>; andrew+netdev@lunn.ch;
> > davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> > pabeni@redhat.com; robh@kernel.org; krzk+dt@kernel.org;
> > conor+dt@kernel.org; Simek, Michal <michal.simek@amd.com>;
> > netdev@vger.kernel.org; devicetree@vger.kernel.org;
> > linux-kernel@vger.kernel.org; linux-arm- kernel@lists.infradead.org;
> > git (AMD-Xilinx) <git@amd.com>; Katakam, Harini
> > <harini.katakam@amd.com>
> > Subject: Re: [PATCH net-next V2 2/2] net: axienet: Add support for
> > 2500base-X only configuration.
> >
> > Caution: This message originated from an External Source. Use proper
> > caution when opening attachments, clicking links, or responding.
> >
> >
> > > This is not an approach that works with the Linux kernel, sorry.
> > >
> > > What we have today is a driver that works for people's hardware -
> > > and we don't know what the capabilities of that hardware is.
> > >
> > > If there's hardware out there today which has XAE_ABILITY_2_5G set,
> > > but defaults to <=1G mode, this will work with the current driver.
> > > However, with your patch applied, it stops working because instead
> > > of the driver indicating MAC_10FD | MAC_100FD | MAC_1000FD, it only
> > > indicates MAC_2500FD. If this happens, it will regress users setups,
> > > and that is something we try not to do.
> > >
> > > Saying "someone else needs to add the code for their FPGA logic"
> > > misses the point - there may not be "someone else" to do that, which
> > > means the only option is to revert your change if it were merged.
> > > That in itself can cause its own user regressions because obviously
> > > stuff that works with this patch stops working.
> > >
> > > This is why we're being cautious, and given your responses, it's not
> > > making Andrew or myself feel that there's a reasonable approach
> > > being taken here.
> > >
> > > >From everything you have said, I am getting the feeling that using
> > > XAE_ABILITY_2_5G to decide which of (1) or (2) is supported is just
> > > wrong. Given that we're talking about an implementation that has
> > > been synthesized at 2.5G and can't operate slower, maybe there's
> > > some way that could be created to specify that in DT?
> > >
> > > e.g. (and I'm sure the DT folk aren't going to like it)...
> > >
> > >       xlnx,axi-ethernet-X.YY.Z-2.5G
> > >
> > > (where X.YY.Z is the version) for implementations that can _only_ do
> > > 2.5G, and leave all other implementations only doing 1G and below.
> > >
> > > Or maybe some DT property. Or something else.
> >
> > Given that AMD has been talking about an FPGA, not silicon, i actually
> > think it would be best to change the IP to explicitly enumerate how it
> > has been synthesised. Make use of some register bits which currently
> > read as 0. Current IP would then remain as 1000BaseX/SGMII,
> > independent of how they have been synthesised. Newer versions of the
> > IP will then set the bits if they have been synthesised as 2) or 3),
> > and the driver can then enable that capability, without breaking
> > current generation systems. Plus there needs to be big fat warning for
> > anybody upgrading to the latest version of the IP for bug fixes to ensure they
> correctly set the synthesis options because it now actually matters.
> >
> >          Andrew
>
> Synthesis options I mentioned in comment might sound confusing, let me clear it up.
> Actual synthesis options (as seen from configuration UI) IP provides are (1) and (2).
> When a user selects (2), IP comes with default 2.5G but also contains 1G
> capabilities which can be enabled and work with by adding switching FPGA logic
> (that makes it (3)).
>
> So, in short  if a user selects (1): It's <=1G only.
> If it selects (2): It's 2.5G only but can be made (3) by FPGA logic changes. So
> whatever existing systems for (3) would be working at default (2).
>
> This is the reason we didn't described (3) in V1 series as that is not provided by IP
> but can be synthesized after FPGA changes.
> Hope I'm able to answer your questions.
>

I understand your concerns that current solution might break if any existing system uses (3).

Russel's suggestion to use DT compatible we can try to send as RFC and check if that is accepted by DT maintainers.
Andrew's suggestion is complete IP solution and will involve IP changes to correct ability register behaviour based on synthesis time selection in a new IP version. But this will need internal discussions and IP rework and might take few months.

Please let me know your thoughts on it.

Re: [PATCH net-next V2 2/2] net: axienet: Add support for 2500base-X only configuration.

[Andrew Lunn]

On Thu, Mar 13, 2025 at 07:34:55AM +0000, Gupta, Suraj wrote:
> [AMD Official Use Only - AMD Internal Distribution Only]
> 
> > -----Original Message-----
> > From: Gupta, Suraj
> > Sent: Thursday, March 13, 2025 9:01 AM
> > To: Andrew Lunn <andrew@lunn.ch>; Russell King (Oracle)
> > <linux@armlinux.org.uk>
> > Cc: Pandey, Radhey Shyam <radhey.shyam.pandey@amd.com>;
> > andrew+netdev@lunn.ch; davem@davemloft.net; edumazet@google.com;
> > kuba@kernel.org; pabeni@redhat.com; robh@kernel.org; krzk+dt@kernel.org;
> > conor+dt@kernel.org; Simek, Michal <michal.simek@amd.com>;
> > netdev@vger.kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> > linux-arm-kernel@lists.infradead.org; git (AMD-Xilinx) <git@amd.com>; Katakam,
> > Harini <harini.katakam@amd.com>
> > Subject: RE: [PATCH net-next V2 2/2] net: axienet: Add support for 2500base-X only
> > configuration.
> >
> >
> >
> > > -----Original Message-----
> > > From: Andrew Lunn <andrew@lunn.ch>
> > > Sent: Thursday, March 13, 2025 3:41 AM
> > > To: Russell King (Oracle) <linux@armlinux.org.uk>
> > > Cc: Gupta, Suraj <Suraj.Gupta2@amd.com>; Pandey, Radhey Shyam
> > > <radhey.shyam.pandey@amd.com>; andrew+netdev@lunn.ch;
> > > davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> > > pabeni@redhat.com; robh@kernel.org; krzk+dt@kernel.org;
> > > conor+dt@kernel.org; Simek, Michal <michal.simek@amd.com>;
> > > netdev@vger.kernel.org; devicetree@vger.kernel.org;
> > > linux-kernel@vger.kernel.org; linux-arm- kernel@lists.infradead.org;
> > > git (AMD-Xilinx) <git@amd.com>; Katakam, Harini
> > > <harini.katakam@amd.com>
> > > Subject: Re: [PATCH net-next V2 2/2] net: axienet: Add support for
> > > 2500base-X only configuration.
> > >
> > > Caution: This message originated from an External Source. Use proper
> > > caution when opening attachments, clicking links, or responding.
> > >
> > >
> > > > This is not an approach that works with the Linux kernel, sorry.
> > > >
> > > > What we have today is a driver that works for people's hardware -
> > > > and we don't know what the capabilities of that hardware is.
> > > >
> > > > If there's hardware out there today which has XAE_ABILITY_2_5G set,
> > > > but defaults to <=1G mode, this will work with the current driver.
> > > > However, with your patch applied, it stops working because instead
> > > > of the driver indicating MAC_10FD | MAC_100FD | MAC_1000FD, it only
> > > > indicates MAC_2500FD. If this happens, it will regress users setups,
> > > > and that is something we try not to do.
> > > >
> > > > Saying "someone else needs to add the code for their FPGA logic"
> > > > misses the point - there may not be "someone else" to do that, which
> > > > means the only option is to revert your change if it were merged.
> > > > That in itself can cause its own user regressions because obviously
> > > > stuff that works with this patch stops working.
> > > >
> > > > This is why we're being cautious, and given your responses, it's not
> > > > making Andrew or myself feel that there's a reasonable approach
> > > > being taken here.
> > > >
> > > > >From everything you have said, I am getting the feeling that using
> > > > XAE_ABILITY_2_5G to decide which of (1) or (2) is supported is just
> > > > wrong. Given that we're talking about an implementation that has
> > > > been synthesized at 2.5G and can't operate slower, maybe there's
> > > > some way that could be created to specify that in DT?
> > > >
> > > > e.g. (and I'm sure the DT folk aren't going to like it)...
> > > >
> > > >       xlnx,axi-ethernet-X.YY.Z-2.5G
> > > >
> > > > (where X.YY.Z is the version) for implementations that can _only_ do
> > > > 2.5G, and leave all other implementations only doing 1G and below.
> > > >
> > > > Or maybe some DT property. Or something else.
> > >
> > > Given that AMD has been talking about an FPGA, not silicon, i actually
> > > think it would be best to change the IP to explicitly enumerate how it
> > > has been synthesised. Make use of some register bits which currently
> > > read as 0. Current IP would then remain as 1000BaseX/SGMII,
> > > independent of how they have been synthesised. Newer versions of the
> > > IP will then set the bits if they have been synthesised as 2) or 3),
> > > and the driver can then enable that capability, without breaking
> > > current generation systems. Plus there needs to be big fat warning for
> > > anybody upgrading to the latest version of the IP for bug fixes to ensure they
> > correctly set the synthesis options because it now actually matters.
> > >
> > >          Andrew
> >
> > Synthesis options I mentioned in comment might sound confusing, let me clear it up.
> > Actual synthesis options (as seen from configuration UI) IP provides are (1) and (2).
> > When a user selects (2), IP comes with default 2.5G but also contains 1G
> > capabilities which can be enabled and work with by adding switching FPGA logic
> > (that makes it (3)).
> >
> > So, in short  if a user selects (1): It's <=1G only.
> > If it selects (2): It's 2.5G only but can be made (3) by FPGA logic changes. So
> > whatever existing systems for (3) would be working at default (2).
> >
> > This is the reason we didn't described (3) in V1 series as that is not provided by IP
> > but can be synthesized after FPGA changes.
> > Hope I'm able to answer your questions.
> >
> 
> I understand your concerns that current solution might break if any existing system uses (3).
> 
> Russel's suggestion to use DT compatible we can try to send as RFC and check if that is accepted by DT maintainers.
> Andrew's suggestion is complete IP solution and will involve IP changes to correct ability register behaviour based on synthesis time selection in a new IP version. But this will need internal discussions and IP rework and might take few months.
> 
> Please let me know your thoughts on it.

Given your comment:

> It's 2.5G only but can be made (3) by FPGA logic changes

It sounds like the design is not ideal at the moment, and you will be
making changes anyway to clean this up. Being fixed in 2.5G mode is
not nice, you need a PHY doing rate adaptation in order to support the
slower speeds.

You should also be thinking forwards. If you add support for fixed
2.5G now, can you cleanly integrate switching between 1000BaseX/SGMII
and 2500BaseX in the future without breaking existing systems? You
probably at least need a paper design of how this will work, so you
can say you have thought through all the user cases: New IP old
driver, Old IP new driver, etc...

	Andrew

RE: [PATCH net-next V2 2/2] net: axienet: Add support for 2500base-X only configuration.

[Gupta, Suraj]

[AMD Official Use Only - AMD Internal Distribution Only]

> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: Thursday, March 13, 2025 6:17 PM
> To: Gupta, Suraj <Suraj.Gupta2@amd.com>
> Cc: Russell King (Oracle) <linux@armlinux.org.uk>; Pandey, Radhey Shyam
> <radhey.shyam.pandey@amd.com>; andrew+netdev@lunn.ch;
> davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> pabeni@redhat.com; robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org;
> Simek, Michal <michal.simek@amd.com>; netdev@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; git (AMD-Xilinx) <git@amd.com>; Katakam, Harini
> <harini.katakam@amd.com>
> Subject: Re: [PATCH net-next V2 2/2] net: axienet: Add support for 2500base-X only
> configuration.
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> On Thu, Mar 13, 2025 at 07:34:55AM +0000, Gupta, Suraj wrote:
> > [AMD Official Use Only - AMD Internal Distribution Only]
> >
> > > -----Original Message-----
> > > From: Gupta, Suraj
> > > Sent: Thursday, March 13, 2025 9:01 AM
> > > To: Andrew Lunn <andrew@lunn.ch>; Russell King (Oracle)
> > > <linux@armlinux.org.uk>
> > > Cc: Pandey, Radhey Shyam <radhey.shyam.pandey@amd.com>;
> > > andrew+netdev@lunn.ch; davem@davemloft.net; edumazet@google.com;
> > > kuba@kernel.org; pabeni@redhat.com; robh@kernel.org;
> > > krzk+dt@kernel.org;
> > > conor+dt@kernel.org; Simek, Michal <michal.simek@amd.com>;
> > > netdev@vger.kernel.org; devicetree@vger.kernel.org;
> > > linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> > > git (AMD-Xilinx) <git@amd.com>; Katakam, Harini
> > > <harini.katakam@amd.com>
> > > Subject: RE: [PATCH net-next V2 2/2] net: axienet: Add support for
> > > 2500base-X only configuration.
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Andrew Lunn <andrew@lunn.ch>
> > > > Sent: Thursday, March 13, 2025 3:41 AM
> > > > To: Russell King (Oracle) <linux@armlinux.org.uk>
> > > > Cc: Gupta, Suraj <Suraj.Gupta2@amd.com>; Pandey, Radhey Shyam
> > > > <radhey.shyam.pandey@amd.com>; andrew+netdev@lunn.ch;
> > > > davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> > > > pabeni@redhat.com; robh@kernel.org; krzk+dt@kernel.org;
> > > > conor+dt@kernel.org; Simek, Michal <michal.simek@amd.com>;
> > > > netdev@vger.kernel.org; devicetree@vger.kernel.org;
> > > > linux-kernel@vger.kernel.org; linux-arm-
> > > > kernel@lists.infradead.org; git (AMD-Xilinx) <git@amd.com>;
> > > > Katakam, Harini <harini.katakam@amd.com>
> > > > Subject: Re: [PATCH net-next V2 2/2] net: axienet: Add support for
> > > > 2500base-X only configuration.
> > > >
> > > > Caution: This message originated from an External Source. Use
> > > > proper caution when opening attachments, clicking links, or responding.
> > > >
> > > >
> > > > > This is not an approach that works with the Linux kernel, sorry.
> > > > >
> > > > > What we have today is a driver that works for people's hardware
> > > > > - and we don't know what the capabilities of that hardware is.
> > > > >
> > > > > If there's hardware out there today which has XAE_ABILITY_2_5G
> > > > > set, but defaults to <=1G mode, this will work with the current driver.
> > > > > However, with your patch applied, it stops working because
> > > > > instead of the driver indicating MAC_10FD | MAC_100FD |
> > > > > MAC_1000FD, it only indicates MAC_2500FD. If this happens, it
> > > > > will regress users setups, and that is something we try not to do.
> > > > >
> > > > > Saying "someone else needs to add the code for their FPGA logic"
> > > > > misses the point - there may not be "someone else" to do that,
> > > > > which means the only option is to revert your change if it were merged.
> > > > > That in itself can cause its own user regressions because
> > > > > obviously stuff that works with this patch stops working.
> > > > >
> > > > > This is why we're being cautious, and given your responses, it's
> > > > > not making Andrew or myself feel that there's a reasonable
> > > > > approach being taken here.
> > > > >
> > > > > >From everything you have said, I am getting the feeling that
> > > > > >using
> > > > > XAE_ABILITY_2_5G to decide which of (1) or (2) is supported is
> > > > > just wrong. Given that we're talking about an implementation
> > > > > that has been synthesized at 2.5G and can't operate slower,
> > > > > maybe there's some way that could be created to specify that in DT?
> > > > >
> > > > > e.g. (and I'm sure the DT folk aren't going to like it)...
> > > > >
> > > > >       xlnx,axi-ethernet-X.YY.Z-2.5G
> > > > >
> > > > > (where X.YY.Z is the version) for implementations that can
> > > > > _only_ do 2.5G, and leave all other implementations only doing 1G and
> below.
> > > > >
> > > > > Or maybe some DT property. Or something else.
> > > >
> > > > Given that AMD has been talking about an FPGA, not silicon, i
> > > > actually think it would be best to change the IP to explicitly
> > > > enumerate how it has been synthesised. Make use of some register
> > > > bits which currently read as 0. Current IP would then remain as
> > > > 1000BaseX/SGMII, independent of how they have been synthesised.
> > > > Newer versions of the IP will then set the bits if they have been
> > > > synthesised as 2) or 3), and the driver can then enable that
> > > > capability, without breaking current generation systems. Plus
> > > > there needs to be big fat warning for anybody upgrading to the
> > > > latest version of the IP for bug fixes to ensure they
> > > correctly set the synthesis options because it now actually matters.
> > > >
> > > >          Andrew
> > >
> > > Synthesis options I mentioned in comment might sound confusing, let me clear it
> up.
> > > Actual synthesis options (as seen from configuration UI) IP provides are (1) and
> (2).
> > > When a user selects (2), IP comes with default 2.5G but also
> > > contains 1G capabilities which can be enabled and work with by
> > > adding switching FPGA logic (that makes it (3)).
> > >
> > > So, in short  if a user selects (1): It's <=1G only.
> > > If it selects (2): It's 2.5G only but can be made (3) by FPGA logic
> > > changes. So whatever existing systems for (3) would be working at default (2).
> > >
> > > This is the reason we didn't described (3) in V1 series as that is
> > > not provided by IP but can be synthesized after FPGA changes.
> > > Hope I'm able to answer your questions.
> > >
> >
> > I understand your concerns that current solution might break if any existing system
> uses (3).
> >
> > Russel's suggestion to use DT compatible we can try to send as RFC and check if
> that is accepted by DT maintainers.
> > Andrew's suggestion is complete IP solution and will involve IP changes to correct
> ability register behaviour based on synthesis time selection in a new IP version. But
> this will need internal discussions and IP rework and might take few months.
> >
> > Please let me know your thoughts on it.
>
> Given your comment:
>
> > It's 2.5G only but can be made (3) by FPGA logic changes
>
> It sounds like the design is not ideal at the moment, and you will be making changes
> anyway to clean this up. Being fixed in 2.5G mode is not nice, you need a PHY
> doing rate adaptation in order to support the slower speeds.
>
> You should also be thinking forwards. If you add support for fixed 2.5G now, can you
> cleanly integrate switching between 1000BaseX/SGMII and 2500BaseX in the future
> without breaking existing systems? You probably at least need a paper design of
> how this will work, so you can say you have thought through all the user cases: New
> IP old driver, Old IP new driver, etc...
>
>         Andrew

Hi Andrew, Russell,

Thank you for your guidance.

Based on our internal discussions, pushing for IP changes would be difficult and time-taking. As an alternative, we propose the following driver modifications to support the current 2.5G-only design while considering existing functionality and potential future switching logic:

1) The driver will advertise speeds based on the Temac ability register-both 1G and 2.5G for the current 2.5G-only design.

2) In axienet_pcs_config(), based on the return type of phylink_mii_c22_pcs_config() (as switching between 1G and 2.5G involves an interface change), if a speed change fails, the driver will print a warning:
"Check for potential missing switching logic in the design if 1G ↔ 2.5G switching is attempted."

Expected Effects:
1) Existing 1G Design
        New Driver:
        (a) Advertises 1G
        (b)If an attempt to change the configuration fails, a warning is printed regarding missing switching logic.

2) 2.5G-Only Design
        Old Driver: Fails, as it attempts to advertise only 1G.
        New Driver:
        (a) Advertises both 1G and 2.5G
        (b)If a configuration change attempt fails, a warning is printed regarding missing switching logic.

3) Custom Design with Switching Logic
        (a) Users will need to implement speed/interface change logic.
        (b) If a configuration change attempt fails, a warning is printed regarding potential missing switching logic.

I understand this approach might generate false warnings if phylink_mii_c22_pcs_config() fails due to other reasons. However, we can explore maintaining current interface / speed in axienet_local structure and track 1G <-> 2.5G switching.

Please let me know your thoughts.

Regards,
Suraj

Re: [PATCH net-next V2 2/2] net: axienet: Add support for 2500base-X only configuration.

[Andrew Lunn]

> Synthesis options I mentioned in comment might sound confusing, let me clear it up.
> Actual synthesis options (as seen from configuration UI) IP provides are (1) and (2). When a user selects (2), IP comes with default 2.5G but also contains 1G capabilities which can be enabled and work with by adding switching FPGA logic (that makes it (3)).
> 
> So, in short  if a user selects (1): It's <=1G only.
> If it selects (2): It's 2.5G only but can be made (3) by FPGA logic changes. So whatever existing systems for (3) would be working at default (2).
> 
> This is the reason we didn't described (3) in V1 series as that is not provided by IP but can be synthesized after FPGA changes.

This whole discussion would of been much easier and wasted a lot less
time if you had given us accurate information right from the start....

So 3) does not exist at the moment, because if it did, the customer
doing it would need to make proprietary changes, both to the licensed
IP and the driver. We have not seen such driver patch submissions...

    Andrew

---
pw-bot: cr