[PATCH v2 01/25] net/axgbe: fix mdio access for non-zero ports and CL45 PHYs

[PATCH v2 01/25] net/axgbe: fix mdio access for non-zero ports and CL45 PHYs

[Venkat Kumar Ande]

The XGBE supports performing MDIO operations using an MDIO command
request. The driver mistakenly uses the mdio port address as the
MDIO command request device address instead of the MDIO command
request port address. Additionally, the driver does not properly check
for and create a clause 45 MDIO command

Without the fix user will get a incorrect value from PHY device.

Check the supplied MDIO register to determine if the request is a clause
45 operation (MII_ADDR_C45). For a clause 45 operation, extract device
address and register number from the supplied MDIO register and use them
to set the MDIO command request device address and register number fields.
For a clause 22 operation, the MDIO request device address is set to zero
and the MDIO command request register number is set to the supplied MDIO
register. In either case, the supplied MDIO port address is used as the
MDIO command request port address.

Fixes: 4ac7516b8b39 ("net/axgbe: add phy init and related APIs")
Cc: stable@dpdk.org

Signed-off-by: Venkat Kumar Ande <venkatkumar.ande@amd.com>
---
 drivers/net/axgbe/axgbe_common.h |  2 --
 drivers/net/axgbe/axgbe_dev.c    | 22 ++++++++++++++++------
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/net/axgbe/axgbe_common.h b/drivers/net/axgbe/axgbe_common.h
index a5d11c5832..51532fb34a 100644
--- a/drivers/net/axgbe/axgbe_common.h
+++ b/drivers/net/axgbe/axgbe_common.h
@@ -407,8 +407,6 @@
 #define MAC_MDIOSCAR_PA_WIDTH		5
 #define MAC_MDIOSCAR_RA_INDEX		0
 #define MAC_MDIOSCAR_RA_WIDTH		16
-#define MAC_MDIOSCAR_REG_INDEX		0
-#define MAC_MDIOSCAR_REG_WIDTH		21
 #define MAC_MDIOSCCDR_BUSY_INDEX	22
 #define MAC_MDIOSCCDR_BUSY_WIDTH	1
 #define MAC_MDIOSCCDR_CMD_INDEX		16
diff --git a/drivers/net/axgbe/axgbe_dev.c b/drivers/net/axgbe/axgbe_dev.c
index 6a7fddffca..3389954aa6 100644
--- a/drivers/net/axgbe/axgbe_dev.c
+++ b/drivers/net/axgbe/axgbe_dev.c
@@ -63,15 +63,27 @@ static int mdio_complete(struct axgbe_port *pdata)
 	return 0;
 }
 
+static unsigned int axgbe_create_mdio_sca(int port, int reg)
+{
+	unsigned int mdio_sca, da;
+
+	da = (reg & MII_ADDR_C45) ? reg >> 16 : 0;
+
+	mdio_sca = 0;
+	AXGMAC_SET_BITS(mdio_sca, MAC_MDIOSCAR, RA, reg);
+	AXGMAC_SET_BITS(mdio_sca, MAC_MDIOSCAR, PA, port);
+	AXGMAC_SET_BITS(mdio_sca, MAC_MDIOSCAR, DA, da);
+
+	return mdio_sca;
+}
+
 static int axgbe_write_ext_mii_regs(struct axgbe_port *pdata, int addr,
 				    int reg, u16 val)
 {
 	unsigned int mdio_sca, mdio_sccd;
 	uint64_t timeout;
 
-	mdio_sca = 0;
-	AXGMAC_SET_BITS(mdio_sca, MAC_MDIOSCAR, REG, reg);
-	AXGMAC_SET_BITS(mdio_sca, MAC_MDIOSCAR, DA, addr);
+	mdio_sca = axgbe_create_mdio_sca(addr, reg);
 	AXGMAC_IOWRITE(pdata, MAC_MDIOSCAR, mdio_sca);
 
 	mdio_sccd = 0;
@@ -97,9 +109,7 @@ static int axgbe_read_ext_mii_regs(struct axgbe_port *pdata, int addr,
 	unsigned int mdio_sca, mdio_sccd;
 	uint64_t timeout;
 
-	mdio_sca = 0;
-	AXGMAC_SET_BITS(mdio_sca, MAC_MDIOSCAR, REG, reg);
-	AXGMAC_SET_BITS(mdio_sca, MAC_MDIOSCAR, DA, addr);
+	mdio_sca = axgbe_create_mdio_sca(addr, reg);
 	AXGMAC_IOWRITE(pdata, MAC_MDIOSCAR, mdio_sca);
 
 	mdio_sccd = 0;
-- 
2.34.1

[PATCH v2 02/25] net/axgbe: reset link when the link never comes back

[Venkat Kumar Ande]

Normally, auto negotiation and reconnect should be automatically done by
the hardware. But there seems to be an issue where auto negotiation has
to be restarted manually. This happens because of link training and so
even though still connected to the partner the link never "comes back".
This needs an auto-negotiation restart.

Without the fix the user will not see the link up status

Fixes: a5c7273771e8 ("net/axgbe: add phy programming APIs")
Cc: stable@dpdk.org

Signed-off-by: Venkat Kumar Ande <venkatkumar.ande@amd.com>
---
 drivers/net/axgbe/axgbe_mdio.c     | 2 +-
 drivers/net/axgbe/axgbe_phy_impl.c | 7 +++++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/net/axgbe/axgbe_mdio.c b/drivers/net/axgbe/axgbe_mdio.c
index 913ceada0d..b03bc471cb 100644
--- a/drivers/net/axgbe/axgbe_mdio.c
+++ b/drivers/net/axgbe/axgbe_mdio.c
@@ -1051,7 +1051,7 @@ static void axgbe_phy_status(struct axgbe_port *pdata)
 							     &an_restart);
 	if (an_restart) {
 		axgbe_phy_config_aneg(pdata);
-		return;
+		goto adjust_link;
 	}
 
 	if (pdata->phy.link) {
diff --git a/drivers/net/axgbe/axgbe_phy_impl.c b/drivers/net/axgbe/axgbe_phy_impl.c
index 44ff28517c..54fe9faae0 100644
--- a/drivers/net/axgbe/axgbe_phy_impl.c
+++ b/drivers/net/axgbe/axgbe_phy_impl.c
@@ -1697,6 +1697,13 @@ static int axgbe_phy_link_status(struct axgbe_port *pdata, int *an_restart)
 	if (reg & MDIO_STAT1_LSTATUS)
 		return 1;
 
+	if (pdata->phy.autoneg == AUTONEG_ENABLE &&
+			phy_data->port_mode == AXGBE_PORT_MODE_BACKPLANE) {
+		if (rte_bit_relaxed_get32(AXGBE_LINK_INIT, &pdata->dev_state)) {
+			*an_restart = 1;
+		}
+	}
+
 	/* No link, attempt a receiver reset cycle */
 	if (phy_data->rrc_count++) {
 		phy_data->rrc_count = 0;
-- 
2.34.1

RE: [PATCH v2 02/25] net/axgbe: reset link when the link never comes back

[Sebastian, Selwin]

[AMD Official Use Only - AMD Internal Distribution Only]

Acked-by: Selwin Sebastian<selwin.sebastian@amd.com>

-----Original Message-----
From: Ande, Venkat Kumar <VenkatKumar.Ande@amd.com>
Sent: Tuesday, May 7, 2024 6:13 PM
To: dev@dpdk.org
Cc: Sebastian, Selwin <Selwin.Sebastian@amd.com>; Ande, Venkat Kumar <VenkatKumar.Ande@amd.com>; stable@dpdk.org
Subject: [PATCH v2 02/25] net/axgbe: reset link when the link never comes back

Normally, auto negotiation and reconnect should be automatically done by the hardware. But there seems to be an issue where auto negotiation has to be restarted manually. This happens because of link training and so even though still connected to the partner the link never "comes back".
This needs an auto-negotiation restart.

Without the fix the user will not see the link up status

Fixes: a5c7273771e8 ("net/axgbe: add phy programming APIs")
Cc: stable@dpdk.org

Signed-off-by: Venkat Kumar Ande <venkatkumar.ande@amd.com>
---
 drivers/net/axgbe/axgbe_mdio.c     | 2 +-
 drivers/net/axgbe/axgbe_phy_impl.c | 7 +++++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/net/axgbe/axgbe_mdio.c b/drivers/net/axgbe/axgbe_mdio.c index 913ceada0d..b03bc471cb 100644
--- a/drivers/net/axgbe/axgbe_mdio.c
+++ b/drivers/net/axgbe/axgbe_mdio.c
@@ -1051,7 +1051,7 @@ static void axgbe_phy_status(struct axgbe_port *pdata)
                                                             &an_restart);
        if (an_restart) {
                axgbe_phy_config_aneg(pdata);
-               return;
+               goto adjust_link;
        }

        if (pdata->phy.link) {
diff --git a/drivers/net/axgbe/axgbe_phy_impl.c b/drivers/net/axgbe/axgbe_phy_impl.c
index 44ff28517c..54fe9faae0 100644
--- a/drivers/net/axgbe/axgbe_phy_impl.c
+++ b/drivers/net/axgbe/axgbe_phy_impl.c
@@ -1697,6 +1697,13 @@ static int axgbe_phy_link_status(struct axgbe_port *pdata, int *an_restart)
        if (reg & MDIO_STAT1_LSTATUS)
                return 1;

+       if (pdata->phy.autoneg == AUTONEG_ENABLE &&
+                       phy_data->port_mode == AXGBE_PORT_MODE_BACKPLANE) {
+               if (rte_bit_relaxed_get32(AXGBE_LINK_INIT, &pdata->dev_state)) {
+                       *an_restart = 1;
+               }
+       }
+
        /* No link, attempt a receiver reset cycle */
        if (phy_data->rrc_count++) {
                phy_data->rrc_count = 0;
--
2.34.1

[PATCH v2 03/25] net/axgbe: fix fluctuations for 1G BELFUSE SFP

[Venkat Kumar Ande]

Frequent link up/down events can happen when a Bel Fuse SFP part is
connected to the amd-xgbe device. Try to avoid the frequent link
issues by resetting the PHY as documented in Bel Fuse SFP datasheets.

Without the fix user will see continuous port link up and down.

Fixes: a5c7273771e8 ("net/axgbe: add phy programming APIs")
Cc: stable@dpdk.org

Signed-off-by: Venkat Kumar Ande <venkatkumar.ande@amd.com>
---
 drivers/net/axgbe/axgbe_phy_impl.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/axgbe/axgbe_phy_impl.c b/drivers/net/axgbe/axgbe_phy_impl.c
index 54fe9faae0..ee9dcbe3da 100644
--- a/drivers/net/axgbe/axgbe_phy_impl.c
+++ b/drivers/net/axgbe/axgbe_phy_impl.c
@@ -578,6 +578,9 @@ static bool axgbe_phy_belfuse_parse_quirks(struct axgbe_port *pdata)
 		   AXGBE_BEL_FUSE_VENDOR, strlen(AXGBE_BEL_FUSE_VENDOR)))
 		return false;
 
+	/* Reset PHY - wait for self-clearing reset bit to clear */
+	pdata->phy_if.phy_impl.reset(pdata);
+
 	if (!memcmp(&sfp_eeprom->base[AXGBE_SFP_BASE_VENDOR_PN],
 		    AXGBE_BEL_FUSE_PARTNO, strlen(AXGBE_BEL_FUSE_PARTNO))) {
 		phy_data->sfp_base = AXGBE_SFP_BASE_1000_SX;
-- 
2.34.1

RE: [PATCH v2 03/25] net/axgbe: fix fluctuations for 1G BELFUSE SFP

[Sebastian, Selwin]

[AMD Official Use Only - AMD Internal Distribution Only]

Acked-by: Selwin Sebastian<selwin.sebastian@amd.com>

-----Original Message-----
From: Ande, Venkat Kumar <VenkatKumar.Ande@amd.com>
Sent: Tuesday, May 7, 2024 6:13 PM
To: dev@dpdk.org
Cc: Sebastian, Selwin <Selwin.Sebastian@amd.com>; Ande, Venkat Kumar <VenkatKumar.Ande@amd.com>; stable@dpdk.org
Subject: [PATCH v2 03/25] net/axgbe: fix fluctuations for 1G BELFUSE SFP

Frequent link up/down events can happen when a Bel Fuse SFP part is connected to the amd-xgbe device. Try to avoid the frequent link issues by resetting the PHY as documented in Bel Fuse SFP datasheets.

Without the fix user will see continuous port link up and down.

Fixes: a5c7273771e8 ("net/axgbe: add phy programming APIs")
Cc: stable@dpdk.org

Signed-off-by: Venkat Kumar Ande <venkatkumar.ande@amd.com>
---
 drivers/net/axgbe/axgbe_phy_impl.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/axgbe/axgbe_phy_impl.c b/drivers/net/axgbe/axgbe_phy_impl.c
index 54fe9faae0..ee9dcbe3da 100644
--- a/drivers/net/axgbe/axgbe_phy_impl.c
+++ b/drivers/net/axgbe/axgbe_phy_impl.c
@@ -578,6 +578,9 @@ static bool axgbe_phy_belfuse_parse_quirks(struct axgbe_port *pdata)
                   AXGBE_BEL_FUSE_VENDOR, strlen(AXGBE_BEL_FUSE_VENDOR)))
                return false;

+       /* Reset PHY - wait for self-clearing reset bit to clear */
+       pdata->phy_if.phy_impl.reset(pdata);
+
        if (!memcmp(&sfp_eeprom->base[AXGBE_SFP_BASE_VENDOR_PN],
                    AXGBE_BEL_FUSE_PARTNO, strlen(AXGBE_BEL_FUSE_PARTNO))) {
                phy_data->sfp_base = AXGBE_SFP_BASE_1000_SX;
--
2.34.1

[PATCH v2 04/25] net/axgbe: update DMA coherency values

[Venkat Kumar Ande]

Based on the IOMMU configuration, the current cache control settings can
result in possible coherency issues. The hardware team has recommended
new settings for the PCI device path to eliminate the issue.

Without the fix the user will get incorrect data in TSO functionality

Fixes: 7c4158a5b592 ("net/axgbe: add DMA programming and start/stop")
Cc: stable@dpdk.org

Signed-off-by: Venkat Kumar Ande <venkatkumar.ande@amd.com>
---
 drivers/net/axgbe/axgbe_dev.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/net/axgbe/axgbe_dev.c b/drivers/net/axgbe/axgbe_dev.c
index 3389954aa6..9b0073eea6 100644
--- a/drivers/net/axgbe/axgbe_dev.c
+++ b/drivers/net/axgbe/axgbe_dev.c
@@ -647,23 +647,21 @@ static void axgbe_config_dma_cache(struct axgbe_port *pdata)
 	unsigned int arcache, awcache, arwcache;
 
 	arcache = 0;
-	AXGMAC_SET_BITS(arcache, DMA_AXIARCR, DRC, 0x3);
+	AXGMAC_SET_BITS(arcache, DMA_AXIARCR, DRC, 0xf);
+	AXGMAC_SET_BITS(arcache, DMA_AXIARCR, TEC, 0xf);
+	AXGMAC_SET_BITS(arcache, DMA_AXIARCR, THC, 0xf);
 	AXGMAC_IOWRITE(pdata, DMA_AXIARCR, arcache);
 
 	awcache = 0;
-	AXGMAC_SET_BITS(awcache, DMA_AXIAWCR, DWC, 0x3);
-	AXGMAC_SET_BITS(awcache, DMA_AXIAWCR, RPC, 0x3);
-	AXGMAC_SET_BITS(awcache, DMA_AXIAWCR, RPD, 0x1);
-	AXGMAC_SET_BITS(awcache, DMA_AXIAWCR, RHC, 0x3);
-	AXGMAC_SET_BITS(awcache, DMA_AXIAWCR, RHD, 0x1);
-	AXGMAC_SET_BITS(awcache, DMA_AXIAWCR, RDC, 0x3);
-	AXGMAC_SET_BITS(awcache, DMA_AXIAWCR, RDD, 0x1);
+	AXGMAC_SET_BITS(awcache, DMA_AXIAWCR, DWC, 0xf);
+	AXGMAC_SET_BITS(awcache, DMA_AXIAWCR, RPC, 0xf);
+	AXGMAC_SET_BITS(awcache, DMA_AXIAWCR, RHC, 0xf);
+	AXGMAC_SET_BITS(awcache, DMA_AXIAWCR, RDC, 0xf);
 	AXGMAC_IOWRITE(pdata, DMA_AXIAWCR, awcache);
 
 	arwcache = 0;
-	AXGMAC_SET_BITS(arwcache, DMA_AXIAWRCR, TDWD, 0x1);
-	AXGMAC_SET_BITS(arwcache, DMA_AXIAWRCR, TDWC, 0x3);
-	AXGMAC_SET_BITS(arwcache, DMA_AXIAWRCR, RDRC, 0x3);
+	AXGMAC_SET_BITS(arwcache, DMA_AXIAWRCR, TDWC, 0xf);
+	AXGMAC_SET_BITS(arwcache, DMA_AXIAWRCR, RDRC, 0xf);
 	AXGMAC_IOWRITE(pdata, DMA_AXIAWRCR, arwcache);
 }
 
-- 
2.34.1

RE: [PATCH v2 04/25] net/axgbe: update DMA coherency values

[Sebastian, Selwin]

[AMD Official Use Only - AMD Internal Distribution Only]

Acked-by: Selwin Sebastian<selwin.sebastian@amd.com>

-----Original Message-----
From: Ande, Venkat Kumar <VenkatKumar.Ande@amd.com>
Sent: Tuesday, May 7, 2024 6:13 PM
To: dev@dpdk.org
Cc: Sebastian, Selwin <Selwin.Sebastian@amd.com>; Ande, Venkat Kumar <VenkatKumar.Ande@amd.com>; stable@dpdk.org
Subject: [PATCH v2 04/25] net/axgbe: update DMA coherency values

Based on the IOMMU configuration, the current cache control settings can result in possible coherency issues. The hardware team has recommended new settings for the PCI device path to eliminate the issue.

Without the fix the user will get incorrect data in TSO functionality

Fixes: 7c4158a5b592 ("net/axgbe: add DMA programming and start/stop")
Cc: stable@dpdk.org

Signed-off-by: Venkat Kumar Ande <venkatkumar.ande@amd.com>
---
 drivers/net/axgbe/axgbe_dev.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/net/axgbe/axgbe_dev.c b/drivers/net/axgbe/axgbe_dev.c index 3389954aa6..9b0073eea6 100644
--- a/drivers/net/axgbe/axgbe_dev.c
+++ b/drivers/net/axgbe/axgbe_dev.c
@@ -647,23 +647,21 @@ static void axgbe_config_dma_cache(struct axgbe_port *pdata)
        unsigned int arcache, awcache, arwcache;

        arcache = 0;
-       AXGMAC_SET_BITS(arcache, DMA_AXIARCR, DRC, 0x3);
+       AXGMAC_SET_BITS(arcache, DMA_AXIARCR, DRC, 0xf);
+       AXGMAC_SET_BITS(arcache, DMA_AXIARCR, TEC, 0xf);
+       AXGMAC_SET_BITS(arcache, DMA_AXIARCR, THC, 0xf);
        AXGMAC_IOWRITE(pdata, DMA_AXIARCR, arcache);

        awcache = 0;
-       AXGMAC_SET_BITS(awcache, DMA_AXIAWCR, DWC, 0x3);
-       AXGMAC_SET_BITS(awcache, DMA_AXIAWCR, RPC, 0x3);
-       AXGMAC_SET_BITS(awcache, DMA_AXIAWCR, RPD, 0x1);
-       AXGMAC_SET_BITS(awcache, DMA_AXIAWCR, RHC, 0x3);
-       AXGMAC_SET_BITS(awcache, DMA_AXIAWCR, RHD, 0x1);
-       AXGMAC_SET_BITS(awcache, DMA_AXIAWCR, RDC, 0x3);
-       AXGMAC_SET_BITS(awcache, DMA_AXIAWCR, RDD, 0x1);
+       AXGMAC_SET_BITS(awcache, DMA_AXIAWCR, DWC, 0xf);
+       AXGMAC_SET_BITS(awcache, DMA_AXIAWCR, RPC, 0xf);
+       AXGMAC_SET_BITS(awcache, DMA_AXIAWCR, RHC, 0xf);
+       AXGMAC_SET_BITS(awcache, DMA_AXIAWCR, RDC, 0xf);
        AXGMAC_IOWRITE(pdata, DMA_AXIAWCR, awcache);

        arwcache = 0;
-       AXGMAC_SET_BITS(arwcache, DMA_AXIAWRCR, TDWD, 0x1);
-       AXGMAC_SET_BITS(arwcache, DMA_AXIAWRCR, TDWC, 0x3);
-       AXGMAC_SET_BITS(arwcache, DMA_AXIAWRCR, RDRC, 0x3);
+       AXGMAC_SET_BITS(arwcache, DMA_AXIAWRCR, TDWC, 0xf);
+       AXGMAC_SET_BITS(arwcache, DMA_AXIAWRCR, RDRC, 0xf);
        AXGMAC_IOWRITE(pdata, DMA_AXIAWRCR, arwcache);  }

--
2.34.1

[PATCH v2 05/25] net/axgbe: disable interrupts during device removal

[Venkat Kumar Ande]

Hardware interrupts are enabled during the init, however,
they are not disabled during close.

Disable all hardware interrupts during close operation to avoid any
issues.

Fixes: 9e890103267e ("net/axgbe: add Rx/Tx setup")
Cc: stable@dpdk.org

Signed-off-by: Venkat Kumar Ande <venkatkumar.ande@amd.com>
---
 drivers/net/axgbe/axgbe_ethdev.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/axgbe/axgbe_ethdev.c b/drivers/net/axgbe/axgbe_ethdev.c
index dd681f15a0..e5d8f7db02 100644
--- a/drivers/net/axgbe/axgbe_ethdev.c
+++ b/drivers/net/axgbe/axgbe_ethdev.c
@@ -2411,12 +2411,14 @@ static int
 axgbe_dev_close(struct rte_eth_dev *eth_dev)
 {
 	struct rte_pci_device *pci_dev;
+	struct axgbe_port *pdata;
 
 	PMD_INIT_FUNC_TRACE();
 
 	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
 		return 0;
 
+	pdata = eth_dev->data->dev_private;
 	pci_dev = RTE_DEV_TO_PCI(eth_dev->device);
 	axgbe_dev_clear_queues(eth_dev);
 
@@ -2426,6 +2428,9 @@ axgbe_dev_close(struct rte_eth_dev *eth_dev)
 				     axgbe_dev_interrupt_handler,
 				     (void *)eth_dev);
 
+	/* Disable all interrupts in the hardware */
+	XP_IOWRITE(pdata, XP_INT_EN, 0x0);
+
 	return 0;
 }
 
-- 
2.34.1

RE: [PATCH v2 05/25] net/axgbe: disable interrupts during device removal

[Sebastian, Selwin]

[AMD Official Use Only - AMD Internal Distribution Only]

Acked-by: Selwin Sebastian<selwin.sebastian@amd.com>

-----Original Message-----
From: Ande, Venkat Kumar <VenkatKumar.Ande@amd.com>
Sent: Tuesday, May 7, 2024 6:13 PM
To: dev@dpdk.org
Cc: Sebastian, Selwin <Selwin.Sebastian@amd.com>; Ande, Venkat Kumar <VenkatKumar.Ande@amd.com>; stable@dpdk.org
Subject: [PATCH v2 05/25] net/axgbe: disable interrupts during device removal

Hardware interrupts are enabled during the init, however, they are not disabled during close.

Disable all hardware interrupts during close operation to avoid any issues.

Fixes: 9e890103267e ("net/axgbe: add Rx/Tx setup")
Cc: stable@dpdk.org

Signed-off-by: Venkat Kumar Ande <venkatkumar.ande@amd.com>
---
 drivers/net/axgbe/axgbe_ethdev.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/axgbe/axgbe_ethdev.c b/drivers/net/axgbe/axgbe_ethdev.c
index dd681f15a0..e5d8f7db02 100644
--- a/drivers/net/axgbe/axgbe_ethdev.c
+++ b/drivers/net/axgbe/axgbe_ethdev.c
@@ -2411,12 +2411,14 @@ static int
 axgbe_dev_close(struct rte_eth_dev *eth_dev)  {
        struct rte_pci_device *pci_dev;
+       struct axgbe_port *pdata;

        PMD_INIT_FUNC_TRACE();

        if (rte_eal_process_type() != RTE_PROC_PRIMARY)
                return 0;

+       pdata = eth_dev->data->dev_private;
        pci_dev = RTE_DEV_TO_PCI(eth_dev->device);
        axgbe_dev_clear_queues(eth_dev);

@@ -2426,6 +2428,9 @@ axgbe_dev_close(struct rte_eth_dev *eth_dev)
                                     axgbe_dev_interrupt_handler,
                                     (void *)eth_dev);

+       /* Disable all interrupts in the hardware */
+       XP_IOWRITE(pdata, XP_INT_EN, 0x0);
+
        return 0;
 }

--
2.34.1

[PATCH v2 06/25] net/axgbe: yellow carp devices do not need rrc

[Venkat Kumar Ande]

Link stability issues are noticed on Yellow carp platforms when Receiver
Reset Cycle is issued. Since the CDR workaround is disabled on these
platforms, the Receiver Reset Cycle is not needed.

So, avoid issuing rrc on Yellow carp platforms.

Without the fix the user will not get the link UP.

Fixes: a5c7273771e8 ("net/axgbe: add phy programming APIs")
Cc: stable@dpdk.org

Signed-off-by: Venkat Kumar Ande <venkatkumar.ande@amd.com>
---
 drivers/net/axgbe/axgbe_ethdev.c   | 5 +++++
 drivers/net/axgbe/axgbe_ethdev.h   | 1 +
 drivers/net/axgbe/axgbe_phy_impl.c | 2 +-
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/axgbe/axgbe_ethdev.c b/drivers/net/axgbe/axgbe_ethdev.c
index e5d8f7db02..86c2ec64e0 100644
--- a/drivers/net/axgbe/axgbe_ethdev.c
+++ b/drivers/net/axgbe/axgbe_ethdev.c
@@ -208,6 +208,7 @@ static struct axgbe_version_data axgbe_v2a = {
 	.ecc_support			= 1,
 	.i2c_support			= 1,
 	.an_cdr_workaround		= 1,
+	.enable_rrc			= 1,
 };
 
 static struct axgbe_version_data axgbe_v2b = {
@@ -220,6 +221,7 @@ static struct axgbe_version_data axgbe_v2b = {
 	.ecc_support			= 1,
 	.i2c_support			= 1,
 	.an_cdr_workaround		= 1,
+	.enable_rrc			= 1,
 };
 
 static const struct rte_eth_desc_lim rx_desc_lim = {
@@ -2269,6 +2271,9 @@ eth_axgbe_dev_init(struct rte_eth_dev *eth_dev)
 
 			/* Yellow Carp devices do not need cdr workaround */
 			pdata->vdata->an_cdr_workaround = 0;
+
+			/* Yellow Carp devices do not need rrc */
+			pdata->vdata->enable_rrc = 0;
 		} else {
 			unknown_cpu = 1;
 		}
diff --git a/drivers/net/axgbe/axgbe_ethdev.h b/drivers/net/axgbe/axgbe_ethdev.h
index df5d63c493..f122dfd516 100644
--- a/drivers/net/axgbe/axgbe_ethdev.h
+++ b/drivers/net/axgbe/axgbe_ethdev.h
@@ -463,6 +463,7 @@ struct axgbe_version_data {
 	unsigned int ecc_support;
 	unsigned int i2c_support;
 	unsigned int an_cdr_workaround;
+	unsigned int enable_rrc;
 };
 
 struct axgbe_mmc_stats {
diff --git a/drivers/net/axgbe/axgbe_phy_impl.c b/drivers/net/axgbe/axgbe_phy_impl.c
index ee9dcbe3da..f51830f800 100644
--- a/drivers/net/axgbe/axgbe_phy_impl.c
+++ b/drivers/net/axgbe/axgbe_phy_impl.c
@@ -1708,7 +1708,7 @@ static int axgbe_phy_link_status(struct axgbe_port *pdata, int *an_restart)
 	}
 
 	/* No link, attempt a receiver reset cycle */
-	if (phy_data->rrc_count++) {
+	if (pdata->vdata->enable_rrc && phy_data->rrc_count++) {
 		phy_data->rrc_count = 0;
 		axgbe_phy_rrc(pdata);
 	}
-- 
2.34.1

RE: [PATCH v2 06/25] net/axgbe: yellow carp devices do not need rrc

[Sebastian, Selwin]

[AMD Official Use Only - AMD Internal Distribution Only]

Acked-by: Selwin Sebastian<selwin.sebastian@amd.com>

-----Original Message-----
From: Ande, Venkat Kumar <VenkatKumar.Ande@amd.com>
Sent: Tuesday, May 7, 2024 6:13 PM
To: dev@dpdk.org
Cc: Sebastian, Selwin <Selwin.Sebastian@amd.com>; Ande, Venkat Kumar <VenkatKumar.Ande@amd.com>; stable@dpdk.org
Subject: [PATCH v2 06/25] net/axgbe: yellow carp devices do not need rrc

Link stability issues are noticed on Yellow carp platforms when Receiver Reset Cycle is issued. Since the CDR workaround is disabled on these platforms, the Receiver Reset Cycle is not needed.

So, avoid issuing rrc on Yellow carp platforms.

Without the fix the user will not get the link UP.

Fixes: a5c7273771e8 ("net/axgbe: add phy programming APIs")
Cc: stable@dpdk.org

Signed-off-by: Venkat Kumar Ande <venkatkumar.ande@amd.com>
---
 drivers/net/axgbe/axgbe_ethdev.c   | 5 +++++
 drivers/net/axgbe/axgbe_ethdev.h   | 1 +
 drivers/net/axgbe/axgbe_phy_impl.c | 2 +-
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/axgbe/axgbe_ethdev.c b/drivers/net/axgbe/axgbe_ethdev.c
index e5d8f7db02..86c2ec64e0 100644
--- a/drivers/net/axgbe/axgbe_ethdev.c
+++ b/drivers/net/axgbe/axgbe_ethdev.c
@@ -208,6 +208,7 @@ static struct axgbe_version_data axgbe_v2a = {
        .ecc_support                    = 1,
        .i2c_support                    = 1,
        .an_cdr_workaround              = 1,
+       .enable_rrc                     = 1,
 };

 static struct axgbe_version_data axgbe_v2b = { @@ -220,6 +221,7 @@ static struct axgbe_version_data axgbe_v2b = {
        .ecc_support                    = 1,
        .i2c_support                    = 1,
        .an_cdr_workaround              = 1,
+       .enable_rrc                     = 1,
 };

 static const struct rte_eth_desc_lim rx_desc_lim = { @@ -2269,6 +2271,9 @@ eth_axgbe_dev_init(struct rte_eth_dev *eth_dev)

                        /* Yellow Carp devices do not need cdr workaround */
                        pdata->vdata->an_cdr_workaround = 0;
+
+                       /* Yellow Carp devices do not need rrc */
+                       pdata->vdata->enable_rrc = 0;
                } else {
                        unknown_cpu = 1;
                }
diff --git a/drivers/net/axgbe/axgbe_ethdev.h b/drivers/net/axgbe/axgbe_ethdev.h
index df5d63c493..f122dfd516 100644
--- a/drivers/net/axgbe/axgbe_ethdev.h
+++ b/drivers/net/axgbe/axgbe_ethdev.h
@@ -463,6 +463,7 @@ struct axgbe_version_data {
        unsigned int ecc_support;
        unsigned int i2c_support;
        unsigned int an_cdr_workaround;
+       unsigned int enable_rrc;
 };

 struct axgbe_mmc_stats {
diff --git a/drivers/net/axgbe/axgbe_phy_impl.c b/drivers/net/axgbe/axgbe_phy_impl.c
index ee9dcbe3da..f51830f800 100644
--- a/drivers/net/axgbe/axgbe_phy_impl.c
+++ b/drivers/net/axgbe/axgbe_phy_impl.c
@@ -1708,7 +1708,7 @@ static int axgbe_phy_link_status(struct axgbe_port *pdata, int *an_restart)
        }

        /* No link, attempt a receiver reset cycle */
-       if (phy_data->rrc_count++) {
+       if (pdata->vdata->enable_rrc && phy_data->rrc_count++) {
                phy_data->rrc_count = 0;
                axgbe_phy_rrc(pdata);
        }
--
2.34.1

[PATCH v2 07/25] net/axgbe: enable PLL control for fixed PHY modes only

[Venkat Kumar Ande]

PLL control setting(RRC) is needed only in fixed PHY configuration to
fix the peer-peer issues. Without the PLL control setting, the link up
takes longer time in a fixed phy configuration.

Without the fix the user will not get the link come UP.

Driver implements SW RRC for Autoneg On configuration, hence PLL control
setting (RRC) is not needed for AN On configuration, and can be skipped.

Also, PLL re-initialization is not needed for PHY Power Off and RRC
commands. Otherwise, they lead to mailbox errors. Added the changes
accordingly.

Fixes: 09b0a36cc7ae ("net/axgbe: toggle PLL settings during rate change")
Cc: stable@dpdk.org

Signed-off-by: Venkat Kumar Ande <venkatkumar.ande@amd.com>
---
 drivers/net/axgbe/axgbe_phy_impl.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/net/axgbe/axgbe_phy_impl.c b/drivers/net/axgbe/axgbe_phy_impl.c
index f51830f800..9c2ae7bba0 100644
--- a/drivers/net/axgbe/axgbe_phy_impl.c
+++ b/drivers/net/axgbe/axgbe_phy_impl.c
@@ -1228,6 +1228,10 @@ static void axgbe_phy_rx_reset(struct axgbe_port *pdata)
 
 static void axgbe_phy_pll_ctrl(struct axgbe_port *pdata, bool enable)
 {
+	/* PLL_CTRL feature needs to be enabled for fixed PHY modes (Non-Autoneg) only */
+	if (pdata->phy.autoneg != AUTONEG_DISABLE)
+		return;
+
 	XMDIO_WRITE_BITS(pdata, MDIO_MMD_PMAPMD, MDIO_VEND2_PMA_MISC_CTRL0,
 			XGBE_PMA_PLL_CTRL_MASK,
 			enable ? XGBE_PMA_PLL_CTRL_SET
@@ -1272,8 +1276,10 @@ static void axgbe_phy_perform_ratechange(struct axgbe_port *pdata,
 	axgbe_phy_rx_reset(pdata);
 
 reenable_pll:
-	 /* Re-enable the PLL control */
-	axgbe_phy_pll_ctrl(pdata, true);
+	/* Enable PLL re-initialization, not needed for PHY Power Off and RRC cmds */
+	if (cmd != AXGBE_MB_CMD_POWER_OFF &&
+	    cmd != AXGBE_MB_CMD_RRC)
+		axgbe_phy_pll_ctrl(pdata, true);
 
 	PMD_DRV_LOG(NOTICE, "firmware mailbox command did not complete\n");
 }
-- 
2.34.1

RE: [PATCH v2 07/25] net/axgbe: enable PLL control for fixed PHY modes only

[Sebastian, Selwin]

[AMD Official Use Only - AMD Internal Distribution Only]

Acked-by: Selwin Sebastian<selwin.sebastian@amd.com>

-----Original Message-----
From: Ande, Venkat Kumar <VenkatKumar.Ande@amd.com>
Sent: Tuesday, May 7, 2024 6:13 PM
To: dev@dpdk.org
Cc: Sebastian, Selwin <Selwin.Sebastian@amd.com>; Ande, Venkat Kumar <VenkatKumar.Ande@amd.com>; stable@dpdk.org
Subject: [PATCH v2 07/25] net/axgbe: enable PLL control for fixed PHY modes only

PLL control setting(RRC) is needed only in fixed PHY configuration to fix the peer-peer issues. Without the PLL control setting, the link up takes longer time in a fixed phy configuration.

Without the fix the user will not get the link come UP.

Driver implements SW RRC for Autoneg On configuration, hence PLL control setting (RRC) is not needed for AN On configuration, and can be skipped.

Also, PLL re-initialization is not needed for PHY Power Off and RRC commands. Otherwise, they lead to mailbox errors. Added the changes accordingly.

Fixes: 09b0a36cc7ae ("net/axgbe: toggle PLL settings during rate change")
Cc: stable@dpdk.org

Signed-off-by: Venkat Kumar Ande <venkatkumar.ande@amd.com>
---
 drivers/net/axgbe/axgbe_phy_impl.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/net/axgbe/axgbe_phy_impl.c b/drivers/net/axgbe/axgbe_phy_impl.c
index f51830f800..9c2ae7bba0 100644
--- a/drivers/net/axgbe/axgbe_phy_impl.c
+++ b/drivers/net/axgbe/axgbe_phy_impl.c
@@ -1228,6 +1228,10 @@ static void axgbe_phy_rx_reset(struct axgbe_port *pdata)

 static void axgbe_phy_pll_ctrl(struct axgbe_port *pdata, bool enable)  {
+       /* PLL_CTRL feature needs to be enabled for fixed PHY modes (Non-Autoneg) only */
+       if (pdata->phy.autoneg != AUTONEG_DISABLE)
+               return;
+
        XMDIO_WRITE_BITS(pdata, MDIO_MMD_PMAPMD, MDIO_VEND2_PMA_MISC_CTRL0,
                        XGBE_PMA_PLL_CTRL_MASK,
                        enable ? XGBE_PMA_PLL_CTRL_SET
@@ -1272,8 +1276,10 @@ static void axgbe_phy_perform_ratechange(struct axgbe_port *pdata,
        axgbe_phy_rx_reset(pdata);

 reenable_pll:
-        /* Re-enable the PLL control */
-       axgbe_phy_pll_ctrl(pdata, true);
+       /* Enable PLL re-initialization, not needed for PHY Power Off and RRC cmds */
+       if (cmd != AXGBE_MB_CMD_POWER_OFF &&
+           cmd != AXGBE_MB_CMD_RRC)
+               axgbe_phy_pll_ctrl(pdata, true);

        PMD_DRV_LOG(NOTICE, "firmware mailbox command did not complete\n");  }
--
2.34.1

Re: [PATCH v2 07/25] net/axgbe: enable PLL control for fixed PHY modes only

[Ferruh Yigit]

On 5/7/2024 1:42 PM, Venkat Kumar Ande wrote:
> PLL control setting(RRC) is needed only in fixed PHY configuration to
> fix the peer-peer issues. Without the PLL control setting, the link up
> takes longer time in a fixed phy configuration.
> 
> Without the fix the user will not get the link come UP.
> 
> Driver implements SW RRC for Autoneg On configuration, hence PLL control
> setting (RRC) is not needed for AN On configuration, and can be skipped.
> 
> Also, PLL re-initialization is not needed for PHY Power Off and RRC
> commands. Otherwise, they lead to mailbox errors. Added the changes
> accordingly.
> 
> Fixes: 09b0a36cc7ae ("net/axgbe: toggle PLL settings during rate change")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Venkat Kumar Ande <venkatkumar.ande@amd.com>
> 

Build fails with this patch.

'AXGBE_MB_CMD_POWER_OFF' & 'AXGBE_MB_CMD_RRC' are missing in the patch,
and they are added later in patch 17/25.


As this is only build fix related to the patch order, please keep Acks
from Selwin in next version (I mean keep ack tags embedded in next
version, so that no need to send new set of acks.)

Thanks,
ferruh

[PATCH v2 08/25] net/axgbe: fix the SFP codes check for DAC cables

[Venkat Kumar Ande]

The current AXGBE code assumes that offset 6 of EEPROM SFP DAC (passive)
cables is NULL. However, some cables (the 5 meter and 7 meter Molex
passive cables) have non-zero data at offset 6. Fix the logic by moving
the passive cable check above the active checks, so as not to be
improperly identified as an active cable. This will fix the issue for
any passive cable that advertises 1000Base-CX in offset 6.

Without the fix the user will not get link UP for few DAC cables.

Fixes: a5c7273771e8 ("net/axgbe: add phy programming APIs")
Cc: stable@dpdk.org

Signed-off-by: Venkat Kumar Ande <venkatkumar.ande@amd.com>
---
 drivers/net/axgbe/axgbe_phy_impl.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/axgbe/axgbe_phy_impl.c b/drivers/net/axgbe/axgbe_phy_impl.c
index 9c2ae7bba0..d4c4a89517 100644
--- a/drivers/net/axgbe/axgbe_phy_impl.c
+++ b/drivers/net/axgbe/axgbe_phy_impl.c
@@ -625,7 +625,10 @@ static void axgbe_phy_sfp_parse_eeprom(struct axgbe_port *pdata)
 	}
 
 	/* Determine the type of SFP */
-	if (sfp_base[AXGBE_SFP_BASE_10GBE_CC] & AXGBE_SFP_BASE_10GBE_CC_SR)
+	if (phy_data->sfp_cable == AXGBE_SFP_CABLE_PASSIVE &&
+		 axgbe_phy_sfp_bit_rate(sfp_eeprom, AXGBE_SFP_SPEED_10000))
+		phy_data->sfp_base = AXGBE_SFP_BASE_10000_CR;
+	else if (sfp_base[AXGBE_SFP_BASE_10GBE_CC] & AXGBE_SFP_BASE_10GBE_CC_SR)
 		phy_data->sfp_base = AXGBE_SFP_BASE_10000_SR;
 	else if (sfp_base[AXGBE_SFP_BASE_10GBE_CC] & AXGBE_SFP_BASE_10GBE_CC_LR)
 		phy_data->sfp_base = AXGBE_SFP_BASE_10000_LR;
@@ -642,9 +645,6 @@ static void axgbe_phy_sfp_parse_eeprom(struct axgbe_port *pdata)
 		phy_data->sfp_base = AXGBE_SFP_BASE_1000_CX;
 	else if (sfp_base[AXGBE_SFP_BASE_1GBE_CC] & AXGBE_SFP_BASE_1GBE_CC_T)
 		phy_data->sfp_base = AXGBE_SFP_BASE_1000_T;
-	else if ((phy_data->sfp_cable == AXGBE_SFP_CABLE_PASSIVE) &&
-		 axgbe_phy_sfp_bit_rate(sfp_eeprom, AXGBE_SFP_SPEED_10000))
-		phy_data->sfp_base = AXGBE_SFP_BASE_10000_CR;
 
 	switch (phy_data->sfp_base) {
 	case AXGBE_SFP_BASE_1000_T:
-- 
2.34.1

RE: [PATCH v2 08/25] net/axgbe: fix the SFP codes check for DAC cables

[Sebastian, Selwin]

[AMD Official Use Only - AMD Internal Distribution Only]

Acked-by: Selwin Sebastian<selwin.sebastian@amd.com>

-----Original Message-----
From: Ande, Venkat Kumar <VenkatKumar.Ande@amd.com>
Sent: Tuesday, May 7, 2024 6:13 PM
To: dev@dpdk.org
Cc: Sebastian, Selwin <Selwin.Sebastian@amd.com>; Ande, Venkat Kumar <VenkatKumar.Ande@amd.com>; stable@dpdk.org
Subject: [PATCH v2 08/25] net/axgbe: fix the SFP codes check for DAC cables

The current AXGBE code assumes that offset 6 of EEPROM SFP DAC (passive) cables is NULL. However, some cables (the 5 meter and 7 meter Molex passive cables) have non-zero data at offset 6. Fix the logic by moving the passive cable check above the active checks, so as not to be improperly identified as an active cable. This will fix the issue for any passive cable that advertises 1000Base-CX in offset 6.

Without the fix the user will not get link UP for few DAC cables.

Fixes: a5c7273771e8 ("net/axgbe: add phy programming APIs")
Cc: stable@dpdk.org

Signed-off-by: Venkat Kumar Ande <venkatkumar.ande@amd.com>
---
 drivers/net/axgbe/axgbe_phy_impl.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/axgbe/axgbe_phy_impl.c b/drivers/net/axgbe/axgbe_phy_impl.c
index 9c2ae7bba0..d4c4a89517 100644
--- a/drivers/net/axgbe/axgbe_phy_impl.c
+++ b/drivers/net/axgbe/axgbe_phy_impl.c
@@ -625,7 +625,10 @@ static void axgbe_phy_sfp_parse_eeprom(struct axgbe_port *pdata)
        }

        /* Determine the type of SFP */
-       if (sfp_base[AXGBE_SFP_BASE_10GBE_CC] & AXGBE_SFP_BASE_10GBE_CC_SR)
+       if (phy_data->sfp_cable == AXGBE_SFP_CABLE_PASSIVE &&
+                axgbe_phy_sfp_bit_rate(sfp_eeprom, AXGBE_SFP_SPEED_10000))
+               phy_data->sfp_base = AXGBE_SFP_BASE_10000_CR;
+       else if (sfp_base[AXGBE_SFP_BASE_10GBE_CC] &
+AXGBE_SFP_BASE_10GBE_CC_SR)
                phy_data->sfp_base = AXGBE_SFP_BASE_10000_SR;
        else if (sfp_base[AXGBE_SFP_BASE_10GBE_CC] & AXGBE_SFP_BASE_10GBE_CC_LR)
                phy_data->sfp_base = AXGBE_SFP_BASE_10000_LR; @@ -642,9 +645,6 @@ static void axgbe_phy_sfp_parse_eeprom(struct axgbe_port *pdata)
                phy_data->sfp_base = AXGBE_SFP_BASE_1000_CX;
        else if (sfp_base[AXGBE_SFP_BASE_1GBE_CC] & AXGBE_SFP_BASE_1GBE_CC_T)
                phy_data->sfp_base = AXGBE_SFP_BASE_1000_T;
-       else if ((phy_data->sfp_cable == AXGBE_SFP_CABLE_PASSIVE) &&
-                axgbe_phy_sfp_bit_rate(sfp_eeprom, AXGBE_SFP_SPEED_10000))
-               phy_data->sfp_base = AXGBE_SFP_BASE_10000_CR;

        switch (phy_data->sfp_base) {
        case AXGBE_SFP_BASE_1000_T:
--
2.34.1

[PATCH v2 09/25] net/axgbe: fix logic around active and passive cables

[Venkat Kumar Ande]

SFP+ active and passive cables are copper cables with fixed SFP+ end
connectors. Due to a misinterpretation of this, SFP+ active cables could
end up not being recognized, causing the driver to fail to establish a
connection.

Introduce a new enum in SFP+ cable types, XGBE_SFP_CABLE_FIBER, that is
the default cable type, and handle active and passive cables when they are
specifically detected.

Fixes: a5c7273771e8 ("net/axgbe: add phy programming APIs")
Cc: stable@dpdk.org

Signed-off-by: Venkat Kumar Ande <venkatkumar.ande@amd.com>
---
 drivers/net/axgbe/axgbe_phy_impl.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/net/axgbe/axgbe_phy_impl.c b/drivers/net/axgbe/axgbe_phy_impl.c
index d4c4a89517..8b66def631 100644
--- a/drivers/net/axgbe/axgbe_phy_impl.c
+++ b/drivers/net/axgbe/axgbe_phy_impl.c
@@ -69,6 +69,7 @@ enum axgbe_sfp_cable {
 	AXGBE_SFP_CABLE_UNKNOWN = 0,
 	AXGBE_SFP_CABLE_ACTIVE,
 	AXGBE_SFP_CABLE_PASSIVE,
+	AXGBE_SFP_CABLE_FIBER,
 };
 
 enum axgbe_sfp_base {
@@ -616,16 +617,18 @@ static void axgbe_phy_sfp_parse_eeprom(struct axgbe_port *pdata)
 
 	axgbe_phy_sfp_parse_quirks(pdata);
 
-	/* Assume ACTIVE cable unless told it is PASSIVE */
+	/* Assume FIBER cable unless told otherwise */
 	if (sfp_base[AXGBE_SFP_BASE_CABLE] & AXGBE_SFP_BASE_CABLE_PASSIVE) {
 		phy_data->sfp_cable = AXGBE_SFP_CABLE_PASSIVE;
 		phy_data->sfp_cable_len = sfp_base[AXGBE_SFP_BASE_CU_CABLE_LEN];
-	} else {
+	} else if (sfp_base[AXGBE_SFP_BASE_CABLE] & AXGBE_SFP_BASE_CABLE_ACTIVE) {
 		phy_data->sfp_cable = AXGBE_SFP_CABLE_ACTIVE;
+	} else {
+		phy_data->sfp_cable = AXGBE_SFP_CABLE_FIBER;
 	}
 
 	/* Determine the type of SFP */
-	if (phy_data->sfp_cable == AXGBE_SFP_CABLE_PASSIVE &&
+	if (phy_data->sfp_cable != AXGBE_SFP_CABLE_FIBER &&
 		 axgbe_phy_sfp_bit_rate(sfp_eeprom, AXGBE_SFP_SPEED_10000))
 		phy_data->sfp_base = AXGBE_SFP_BASE_10000_CR;
 	else if (sfp_base[AXGBE_SFP_BASE_10GBE_CC] & AXGBE_SFP_BASE_10GBE_CC_SR)
-- 
2.34.1

RE: [PATCH v2 09/25] net/axgbe: fix logic around active and passive cables

[Sebastian, Selwin]

[AMD Official Use Only - AMD Internal Distribution Only]

Acked-by: Selwin Sebastian<selwin.sebastian@amd.com>

-----Original Message-----
From: Ande, Venkat Kumar <VenkatKumar.Ande@amd.com>
Sent: Tuesday, May 7, 2024 6:13 PM
To: dev@dpdk.org
Cc: Sebastian, Selwin <Selwin.Sebastian@amd.com>; Ande, Venkat Kumar <VenkatKumar.Ande@amd.com>; stable@dpdk.org
Subject: [PATCH v2 09/25] net/axgbe: fix logic around active and passive cables

SFP+ active and passive cables are copper cables with fixed SFP+ end
connectors. Due to a misinterpretation of this, SFP+ active cables could end up not being recognized, causing the driver to fail to establish a connection.

Introduce a new enum in SFP+ cable types, XGBE_SFP_CABLE_FIBER, that is the default cable type, and handle active and passive cables when they are specifically detected.

Fixes: a5c7273771e8 ("net/axgbe: add phy programming APIs")
Cc: stable@dpdk.org

Signed-off-by: Venkat Kumar Ande <venkatkumar.ande@amd.com>
---
 drivers/net/axgbe/axgbe_phy_impl.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/net/axgbe/axgbe_phy_impl.c b/drivers/net/axgbe/axgbe_phy_impl.c
index d4c4a89517..8b66def631 100644
--- a/drivers/net/axgbe/axgbe_phy_impl.c
+++ b/drivers/net/axgbe/axgbe_phy_impl.c
@@ -69,6 +69,7 @@ enum axgbe_sfp_cable {
        AXGBE_SFP_CABLE_UNKNOWN = 0,
        AXGBE_SFP_CABLE_ACTIVE,
        AXGBE_SFP_CABLE_PASSIVE,
+       AXGBE_SFP_CABLE_FIBER,
 };

 enum axgbe_sfp_base {
@@ -616,16 +617,18 @@ static void axgbe_phy_sfp_parse_eeprom(struct axgbe_port *pdata)

        axgbe_phy_sfp_parse_quirks(pdata);

-       /* Assume ACTIVE cable unless told it is PASSIVE */
+       /* Assume FIBER cable unless told otherwise */
        if (sfp_base[AXGBE_SFP_BASE_CABLE] & AXGBE_SFP_BASE_CABLE_PASSIVE) {
                phy_data->sfp_cable = AXGBE_SFP_CABLE_PASSIVE;
                phy_data->sfp_cable_len = sfp_base[AXGBE_SFP_BASE_CU_CABLE_LEN];
-       } else {
+       } else if (sfp_base[AXGBE_SFP_BASE_CABLE] &
+AXGBE_SFP_BASE_CABLE_ACTIVE) {
                phy_data->sfp_cable = AXGBE_SFP_CABLE_ACTIVE;
+       } else {
+               phy_data->sfp_cable = AXGBE_SFP_CABLE_FIBER;
        }

        /* Determine the type of SFP */
-       if (phy_data->sfp_cable == AXGBE_SFP_CABLE_PASSIVE &&
+       if (phy_data->sfp_cable != AXGBE_SFP_CABLE_FIBER &&
                 axgbe_phy_sfp_bit_rate(sfp_eeprom, AXGBE_SFP_SPEED_10000))
                phy_data->sfp_base = AXGBE_SFP_BASE_10000_CR;
        else if (sfp_base[AXGBE_SFP_BASE_10GBE_CC] & AXGBE_SFP_BASE_10GBE_CC_SR)
--
2.34.1

[PATCH v2 10/25] net/axgbe: check only the minimum speed for cables

[Venkat Kumar Ande]

There are cables that exist that can support speeds in excess of 10GbE.
The driver, however, restricts the EEPROM advertised nominal bitrate to
a specific range, which can prevent usage of cables that can support,
for example, up to 25GbE.

Rather than checking that an active or passive cable supports a specific
range, only check for a minimum supported speed.

Fixes: a5c7273771e8 ("net/axgbe: add phy programming APIs")
Cc: stable@dpdk.org

Signed-off-by: Venkat Kumar Ande <venkatkumar.ande@amd.com>
---
 drivers/net/axgbe/axgbe_phy_impl.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/net/axgbe/axgbe_phy_impl.c b/drivers/net/axgbe/axgbe_phy_impl.c
index 8b66def631..0ad5b4ba85 100644
--- a/drivers/net/axgbe/axgbe_phy_impl.c
+++ b/drivers/net/axgbe/axgbe_phy_impl.c
@@ -117,9 +117,7 @@ enum axgbe_sfp_speed {
 
 #define AXGBE_SFP_BASE_BR			12
 #define AXGBE_SFP_BASE_BR_1GBE_MIN		0x0a
-#define AXGBE_SFP_BASE_BR_1GBE_MAX		0x0d
 #define AXGBE_SFP_BASE_BR_10GBE_MIN		0x64
-#define AXGBE_SFP_BASE_BR_10GBE_MAX		0x68
 
 #define AXGBE_SFP_BASE_CU_CABLE_LEN		18
 
@@ -536,25 +534,22 @@ static void axgbe_phy_sfp_phy_settings(struct axgbe_port *pdata)
 static bool axgbe_phy_sfp_bit_rate(struct axgbe_sfp_eeprom *sfp_eeprom,
 				   enum axgbe_sfp_speed sfp_speed)
 {
-	u8 *sfp_base, min, max;
+	u8 *sfp_base, min;
 
 	sfp_base = sfp_eeprom->base;
 
 	switch (sfp_speed) {
 	case AXGBE_SFP_SPEED_1000:
 		min = AXGBE_SFP_BASE_BR_1GBE_MIN;
-		max = AXGBE_SFP_BASE_BR_1GBE_MAX;
 		break;
 	case AXGBE_SFP_SPEED_10000:
 		min = AXGBE_SFP_BASE_BR_10GBE_MIN;
-		max = AXGBE_SFP_BASE_BR_10GBE_MAX;
 		break;
 	default:
 		return false;
 	}
 
-	return ((sfp_base[AXGBE_SFP_BASE_BR] >= min) &&
-		(sfp_base[AXGBE_SFP_BASE_BR] <= max));
+	return sfp_base[AXGBE_SFP_BASE_BR] >= min;
 }
 
 static void axgbe_phy_sfp_external_phy(struct axgbe_port *pdata)
-- 
2.34.1

RE: [PATCH v2 10/25] net/axgbe: check only the minimum speed for cables

[Sebastian, Selwin]

[AMD Official Use Only - AMD Internal Distribution Only]

Acked-by: Selwin Sebastian<selwin.sebastian@amd.com>

-----Original Message-----
From: Ande, Venkat Kumar <VenkatKumar.Ande@amd.com>
Sent: Tuesday, May 7, 2024 6:13 PM
To: dev@dpdk.org
Cc: Sebastian, Selwin <Selwin.Sebastian@amd.com>; Ande, Venkat Kumar <VenkatKumar.Ande@amd.com>; stable@dpdk.org
Subject: [PATCH v2 10/25] net/axgbe: check only the minimum speed for cables

There are cables that exist that can support speeds in excess of 10GbE.
The driver, however, restricts the EEPROM advertised nominal bitrate to a specific range, which can prevent usage of cables that can support, for example, up to 25GbE.

Rather than checking that an active or passive cable supports a specific range, only check for a minimum supported speed.

Fixes: a5c7273771e8 ("net/axgbe: add phy programming APIs")
Cc: stable@dpdk.org

Signed-off-by: Venkat Kumar Ande <venkatkumar.ande@amd.com>
---
 drivers/net/axgbe/axgbe_phy_impl.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/net/axgbe/axgbe_phy_impl.c b/drivers/net/axgbe/axgbe_phy_impl.c
index 8b66def631..0ad5b4ba85 100644
--- a/drivers/net/axgbe/axgbe_phy_impl.c
+++ b/drivers/net/axgbe/axgbe_phy_impl.c
@@ -117,9 +117,7 @@ enum axgbe_sfp_speed {

 #define AXGBE_SFP_BASE_BR                      12
 #define AXGBE_SFP_BASE_BR_1GBE_MIN             0x0a
-#define AXGBE_SFP_BASE_BR_1GBE_MAX             0x0d
 #define AXGBE_SFP_BASE_BR_10GBE_MIN            0x64
-#define AXGBE_SFP_BASE_BR_10GBE_MAX            0x68

 #define AXGBE_SFP_BASE_CU_CABLE_LEN            18

@@ -536,25 +534,22 @@ static void axgbe_phy_sfp_phy_settings(struct axgbe_port *pdata)  static bool axgbe_phy_sfp_bit_rate(struct axgbe_sfp_eeprom *sfp_eeprom,
                                   enum axgbe_sfp_speed sfp_speed)
 {
-       u8 *sfp_base, min, max;
+       u8 *sfp_base, min;

        sfp_base = sfp_eeprom->base;

        switch (sfp_speed) {
        case AXGBE_SFP_SPEED_1000:
                min = AXGBE_SFP_BASE_BR_1GBE_MIN;
-               max = AXGBE_SFP_BASE_BR_1GBE_MAX;
                break;
        case AXGBE_SFP_SPEED_10000:
                min = AXGBE_SFP_BASE_BR_10GBE_MIN;
-               max = AXGBE_SFP_BASE_BR_10GBE_MAX;
                break;
        default:
                return false;
        }

-       return ((sfp_base[AXGBE_SFP_BASE_BR] >= min) &&
-               (sfp_base[AXGBE_SFP_BASE_BR] <= max));
+       return sfp_base[AXGBE_SFP_BASE_BR] >= min;
 }

 static void axgbe_phy_sfp_external_phy(struct axgbe_port *pdata)
--
2.34.1

[PATCH v2 11/25] net/axgbe: flow Tx Ctrl Registers are h/w version dependent

[Venkat Kumar Ande]

There is difference in the TX Flow Control registers (TFCR) between the
revisions of the hardware. The older revisions of hardware used to have
single register per queue. Whereas, the newer revision of hardware (from
ver 30H onwards) have one register per priority.

Without the fix the user will face problem in TX operation on new 30H HW

Fixes: 7c4158a5b592 ("net/axgbe: add DMA programming and start/stop")
Cc: stable@dpdk.org

Signed-off-by: Venkat Kumar Ande <venkatkumar.ande@amd.com>
---
 drivers/net/axgbe/axgbe_dev.c | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/drivers/net/axgbe/axgbe_dev.c b/drivers/net/axgbe/axgbe_dev.c
index 9b0073eea6..5233633a53 100644
--- a/drivers/net/axgbe/axgbe_dev.c
+++ b/drivers/net/axgbe/axgbe_dev.c
@@ -269,20 +269,28 @@ static int axgbe_set_speed(struct axgbe_port *pdata, int speed)
 	return 0;
 }
 
+static unsigned int axgbe_get_fc_queue_count(struct axgbe_port *pdata)
+{
+	unsigned int max_q_count = AXGMAC_MAX_FLOW_CONTROL_QUEUES;
+
+	/* From MAC ver 30H the TFCR is per priority, instead of per queue */
+	if (AXGMAC_GET_BITS(pdata->hw_feat.version, MAC_VR, SNPSVER) >= 0x30)
+		return max_q_count;
+	else
+		return (RTE_MIN(pdata->tx_q_count, max_q_count));
+}
+
 static int axgbe_disable_tx_flow_control(struct axgbe_port *pdata)
 {
-	unsigned int max_q_count, q_count;
 	unsigned int reg, reg_val;
-	unsigned int i;
+	unsigned int i, q_count;
 
 	/* Clear MTL flow control */
 	for (i = 0; i < pdata->rx_q_count; i++)
 		AXGMAC_MTL_IOWRITE_BITS(pdata, i, MTL_Q_RQOMR, EHFC, 0);
 
 	/* Clear MAC flow control */
-	max_q_count = AXGMAC_MAX_FLOW_CONTROL_QUEUES;
-	q_count = RTE_MIN(pdata->tx_q_count,
-			max_q_count);
+	q_count = axgbe_get_fc_queue_count(pdata);
 	reg = MAC_Q0TFCR;
 	for (i = 0; i < q_count; i++) {
 		reg_val = AXGMAC_IOREAD(pdata, reg);
@@ -297,9 +305,8 @@ static int axgbe_disable_tx_flow_control(struct axgbe_port *pdata)
 
 static int axgbe_enable_tx_flow_control(struct axgbe_port *pdata)
 {
-	unsigned int max_q_count, q_count;
 	unsigned int reg, reg_val;
-	unsigned int i;
+	unsigned int i, q_count;
 
 	/* Set MTL flow control */
 	for (i = 0; i < pdata->rx_q_count; i++) {
@@ -316,9 +323,7 @@ static int axgbe_enable_tx_flow_control(struct axgbe_port *pdata)
 	}
 
 	/* Set MAC flow control */
-	max_q_count = AXGMAC_MAX_FLOW_CONTROL_QUEUES;
-	q_count = RTE_MIN(pdata->tx_q_count,
-			max_q_count);
+	q_count = axgbe_get_fc_queue_count(pdata);
 	reg = MAC_Q0TFCR;
 	for (i = 0; i < q_count; i++) {
 		reg_val = AXGMAC_IOREAD(pdata, reg);
-- 
2.34.1

RE: [PATCH v2 11/25] net/axgbe: flow Tx Ctrl Registers are h/w version dependent

[Sebastian, Selwin]

[AMD Official Use Only - AMD Internal Distribution Only]

Acked-by: Selwin Sebastian<selwin.sebastian@amd.com>

-----Original Message-----
From: Ande, Venkat Kumar <VenkatKumar.Ande@amd.com>
Sent: Tuesday, May 7, 2024 6:13 PM
To: dev@dpdk.org
Cc: Sebastian, Selwin <Selwin.Sebastian@amd.com>; Ande, Venkat Kumar <VenkatKumar.Ande@amd.com>; stable@dpdk.org
Subject: [PATCH v2 11/25] net/axgbe: flow Tx Ctrl Registers are h/w version dependent

There is difference in the TX Flow Control registers (TFCR) between the revisions of the hardware. The older revisions of hardware used to have single register per queue. Whereas, the newer revision of hardware (from ver 30H onwards) have one register per priority.

Without the fix the user will face problem in TX operation on new 30H HW

Fixes: 7c4158a5b592 ("net/axgbe: add DMA programming and start/stop")
Cc: stable@dpdk.org

Signed-off-by: Venkat Kumar Ande <venkatkumar.ande@amd.com>
---
 drivers/net/axgbe/axgbe_dev.c | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/drivers/net/axgbe/axgbe_dev.c b/drivers/net/axgbe/axgbe_dev.c index 9b0073eea6..5233633a53 100644
--- a/drivers/net/axgbe/axgbe_dev.c
+++ b/drivers/net/axgbe/axgbe_dev.c
@@ -269,20 +269,28 @@ static int axgbe_set_speed(struct axgbe_port *pdata, int speed)
        return 0;
 }

+static unsigned int axgbe_get_fc_queue_count(struct axgbe_port *pdata)
+{
+       unsigned int max_q_count = AXGMAC_MAX_FLOW_CONTROL_QUEUES;
+
+       /* From MAC ver 30H the TFCR is per priority, instead of per queue */
+       if (AXGMAC_GET_BITS(pdata->hw_feat.version, MAC_VR, SNPSVER) >= 0x30)
+               return max_q_count;
+       else
+               return (RTE_MIN(pdata->tx_q_count, max_q_count)); }
+
 static int axgbe_disable_tx_flow_control(struct axgbe_port *pdata)  {
-       unsigned int max_q_count, q_count;
        unsigned int reg, reg_val;
-       unsigned int i;
+       unsigned int i, q_count;

        /* Clear MTL flow control */
        for (i = 0; i < pdata->rx_q_count; i++)
                AXGMAC_MTL_IOWRITE_BITS(pdata, i, MTL_Q_RQOMR, EHFC, 0);

        /* Clear MAC flow control */
-       max_q_count = AXGMAC_MAX_FLOW_CONTROL_QUEUES;
-       q_count = RTE_MIN(pdata->tx_q_count,
-                       max_q_count);
+       q_count = axgbe_get_fc_queue_count(pdata);
        reg = MAC_Q0TFCR;
        for (i = 0; i < q_count; i++) {
                reg_val = AXGMAC_IOREAD(pdata, reg);
@@ -297,9 +305,8 @@ static int axgbe_disable_tx_flow_control(struct axgbe_port *pdata)

 static int axgbe_enable_tx_flow_control(struct axgbe_port *pdata)  {
-       unsigned int max_q_count, q_count;
        unsigned int reg, reg_val;
-       unsigned int i;
+       unsigned int i, q_count;

        /* Set MTL flow control */
        for (i = 0; i < pdata->rx_q_count; i++) { @@ -316,9 +323,7 @@ static int axgbe_enable_tx_flow_control(struct axgbe_port *pdata)
        }

        /* Set MAC flow control */
-       max_q_count = AXGMAC_MAX_FLOW_CONTROL_QUEUES;
-       q_count = RTE_MIN(pdata->tx_q_count,
-                       max_q_count);
+       q_count = axgbe_get_fc_queue_count(pdata);
        reg = MAC_Q0TFCR;
        for (i = 0; i < q_count; i++) {
                reg_val = AXGMAC_IOREAD(pdata, reg);
--
2.34.1

[PATCH v2 12/25] net/axgbe: delay AN timeout during KR training

[Venkat Kumar Ande]

AN restart triggered during KR training not only aborts the KR training
process but also move the HW to unstable state. Driver has to wait up to
500ms or until the KR training is completed before restarting AN cycle.

Without the fix the user will face KR training failure issues.

Fixes: a5c7273771e8 ("net/axgbe: add phy programming APIs")
Cc: stable@dpdk.org

Signed-off-by: Venkat Kumar Ande <venkatkumar.ande@amd.com>
---
 drivers/net/axgbe/axgbe_ethdev.h |  2 ++
 drivers/net/axgbe/axgbe_mdio.c   | 25 +++++++++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/drivers/net/axgbe/axgbe_ethdev.h b/drivers/net/axgbe/axgbe_ethdev.h
index f122dfd516..d1fac667c2 100644
--- a/drivers/net/axgbe/axgbe_ethdev.h
+++ b/drivers/net/axgbe/axgbe_ethdev.h
@@ -111,6 +111,7 @@
 /* Auto-negotiation */
 #define AXGBE_AN_MS_TIMEOUT		500
 #define AXGBE_LINK_TIMEOUT		5
+#define AXGBE_KR_TRAINING_WAIT_ITER	50
 
 #define AXGBE_SGMII_AN_LINK_STATUS	BIT(1)
 #define AXGBE_SGMII_AN_LINK_SPEED	(BIT(2) | BIT(3))
@@ -661,6 +662,7 @@ struct axgbe_port {
 	unsigned int parallel_detect;
 	unsigned int fec_ability;
 	unsigned long an_start;
+	unsigned long kr_start_time;
 	enum axgbe_an_mode an_mode;
 
 	/* I2C support */
diff --git a/drivers/net/axgbe/axgbe_mdio.c b/drivers/net/axgbe/axgbe_mdio.c
index b03bc471cb..e1a737cf62 100644
--- a/drivers/net/axgbe/axgbe_mdio.c
+++ b/drivers/net/axgbe/axgbe_mdio.c
@@ -357,6 +357,7 @@ static enum axgbe_an axgbe_an73_tx_training(struct axgbe_port *pdata,
 	reg |= AXGBE_KR_TRAINING_ENABLE;
 	reg |= AXGBE_KR_TRAINING_START;
 	XMDIO_WRITE(pdata, MDIO_MMD_PMAPMD, MDIO_PMA_10GBR_PMD_CTRL, reg);
+	pdata->kr_start_time = rte_get_timer_cycles();
 
 	PMD_DRV_LOG(DEBUG, "KR training initiated\n");
 	if (pdata->phy_if.phy_impl.kr_training_post)
@@ -487,6 +488,7 @@ static enum axgbe_an axgbe_an73_incompat_link(struct axgbe_port *pdata)
 
 	axgbe_an_disable(pdata);
 	axgbe_switch_mode(pdata);
+	pdata->an_result = AXGBE_AN_READY;
 	axgbe_an_restart(pdata);
 
 	return AXGBE_AN_INCOMPAT_LINK;
@@ -967,11 +969,34 @@ static void axgbe_check_link_timeout(struct axgbe_port *pdata)
 {
 	unsigned long link_timeout;
 	unsigned long ticks;
+	unsigned long kr_time;
+	int wait;
 
 	link_timeout = pdata->link_check + (AXGBE_LINK_TIMEOUT *
 					    2 *  rte_get_timer_hz());
 	ticks = rte_get_timer_cycles();
 	if (time_after(ticks, link_timeout)) {
+		if ((axgbe_cur_mode(pdata) == AXGBE_MODE_KR) &&
+		    pdata->phy.autoneg == AUTONEG_ENABLE) {
+			/* AN restart should not happen while KR training is in progress.
+			 * The while loop ensures no AN restart during KR training,
+			 * waits up to 500ms and AN restart is triggered only if KR
+			 * training is failed.
+			 */
+			wait = AXGBE_KR_TRAINING_WAIT_ITER;
+			while (wait--) {
+				kr_time = pdata->kr_start_time +
+					  msecs_to_timer_cycles(AXGBE_AN_MS_TIMEOUT);
+				ticks = rte_get_timer_cycles();
+				if (time_after(ticks, kr_time))
+					break;
+				/* AN restart is not required, if AN result is COMPLETE */
+				if (pdata->an_result == AXGBE_AN_COMPLETE)
+					return;
+				rte_delay_us(10500);
+			}
+		}
+
 		PMD_DRV_LOG(NOTICE, "AN link timeout\n");
 		axgbe_phy_config_aneg(pdata);
 	}
-- 
2.34.1

RE: [PATCH v2 12/25] net/axgbe: delay AN timeout during KR training

[Sebastian, Selwin]

[AMD Official Use Only - AMD Internal Distribution Only]

Acked-by: Selwin Sebastian<selwin.sebastian@amd.com>

-----Original Message-----
From: Ande, Venkat Kumar <VenkatKumar.Ande@amd.com>
Sent: Tuesday, May 7, 2024 6:13 PM
To: dev@dpdk.org
Cc: Sebastian, Selwin <Selwin.Sebastian@amd.com>; Ande, Venkat Kumar <VenkatKumar.Ande@amd.com>; stable@dpdk.org
Subject: [PATCH v2 12/25] net/axgbe: delay AN timeout during KR training

AN restart triggered during KR training not only aborts the KR training process but also move the HW to unstable state. Driver has to wait up to 500ms or until the KR training is completed before restarting AN cycle.

Without the fix the user will face KR training failure issues.

Fixes: a5c7273771e8 ("net/axgbe: add phy programming APIs")
Cc: stable@dpdk.org

Signed-off-by: Venkat Kumar Ande <venkatkumar.ande@amd.com>
---
 drivers/net/axgbe/axgbe_ethdev.h |  2 ++
 drivers/net/axgbe/axgbe_mdio.c   | 25 +++++++++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/drivers/net/axgbe/axgbe_ethdev.h b/drivers/net/axgbe/axgbe_ethdev.h
index f122dfd516..d1fac667c2 100644
--- a/drivers/net/axgbe/axgbe_ethdev.h
+++ b/drivers/net/axgbe/axgbe_ethdev.h
@@ -111,6 +111,7 @@
 /* Auto-negotiation */
 #define AXGBE_AN_MS_TIMEOUT            500
 #define AXGBE_LINK_TIMEOUT             5
+#define AXGBE_KR_TRAINING_WAIT_ITER    50

 #define AXGBE_SGMII_AN_LINK_STATUS     BIT(1)
 #define AXGBE_SGMII_AN_LINK_SPEED      (BIT(2) | BIT(3))
@@ -661,6 +662,7 @@ struct axgbe_port {
        unsigned int parallel_detect;
        unsigned int fec_ability;
        unsigned long an_start;
+       unsigned long kr_start_time;
        enum axgbe_an_mode an_mode;

        /* I2C support */
diff --git a/drivers/net/axgbe/axgbe_mdio.c b/drivers/net/axgbe/axgbe_mdio.c index b03bc471cb..e1a737cf62 100644
--- a/drivers/net/axgbe/axgbe_mdio.c
+++ b/drivers/net/axgbe/axgbe_mdio.c
@@ -357,6 +357,7 @@ static enum axgbe_an axgbe_an73_tx_training(struct axgbe_port *pdata,
        reg |= AXGBE_KR_TRAINING_ENABLE;
        reg |= AXGBE_KR_TRAINING_START;
        XMDIO_WRITE(pdata, MDIO_MMD_PMAPMD, MDIO_PMA_10GBR_PMD_CTRL, reg);
+       pdata->kr_start_time = rte_get_timer_cycles();

        PMD_DRV_LOG(DEBUG, "KR training initiated\n");
        if (pdata->phy_if.phy_impl.kr_training_post)
@@ -487,6 +488,7 @@ static enum axgbe_an axgbe_an73_incompat_link(struct axgbe_port *pdata)

        axgbe_an_disable(pdata);
        axgbe_switch_mode(pdata);
+       pdata->an_result = AXGBE_AN_READY;
        axgbe_an_restart(pdata);

        return AXGBE_AN_INCOMPAT_LINK;
@@ -967,11 +969,34 @@ static void axgbe_check_link_timeout(struct axgbe_port *pdata)  {
        unsigned long link_timeout;
        unsigned long ticks;
+       unsigned long kr_time;
+       int wait;

        link_timeout = pdata->link_check + (AXGBE_LINK_TIMEOUT *
                                            2 *  rte_get_timer_hz());
        ticks = rte_get_timer_cycles();
        if (time_after(ticks, link_timeout)) {
+               if ((axgbe_cur_mode(pdata) == AXGBE_MODE_KR) &&
+                   pdata->phy.autoneg == AUTONEG_ENABLE) {
+                       /* AN restart should not happen while KR training is in progress.
+                        * The while loop ensures no AN restart during KR training,
+                        * waits up to 500ms and AN restart is triggered only if KR
+                        * training is failed.
+                        */
+                       wait = AXGBE_KR_TRAINING_WAIT_ITER;
+                       while (wait--) {
+                               kr_time = pdata->kr_start_time +
+                                         msecs_to_timer_cycles(AXGBE_AN_MS_TIMEOUT);
+                               ticks = rte_get_timer_cycles();
+                               if (time_after(ticks, kr_time))
+                                       break;
+                               /* AN restart is not required, if AN result is COMPLETE */
+                               if (pdata->an_result == AXGBE_AN_COMPLETE)
+                                       return;
+                               rte_delay_us(10500);
+                       }
+               }
+
                PMD_DRV_LOG(NOTICE, "AN link timeout\n");
                axgbe_phy_config_aneg(pdata);
        }
--
2.34.1

[PATCH v2 23/25] net/axgbe: fix the false linkup in axgbe PHY status

[Venkat Kumar Ande]

In the event of a change in AXGBE mode, the current auto-negotiation
needs to be reset and the AN cycle needs to be re-triggered. However,
the current code ignores the return value of axgbe_set_mode(), leading to
false information as the link is declared without checking the status
register.

Fix this by propagating the mode switch status information to
axgbe_phy_status().

Fixes: 102b6ec3d5c3 ("net/axgbe: support auto-negotiation for 1Gbps")
Cc: stable@dpdk.org

Signed-off-by: Venkat Kumar Ande <venkatkumar.ande@amd.com>
---
 drivers/net/axgbe/axgbe_mdio.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/net/axgbe/axgbe_mdio.c b/drivers/net/axgbe/axgbe_mdio.c
index 9fe30e83bc..130a67479e 100644
--- a/drivers/net/axgbe/axgbe_mdio.c
+++ b/drivers/net/axgbe/axgbe_mdio.c
@@ -1028,7 +1028,7 @@ static enum axgbe_mode axgbe_phy_status_aneg(struct axgbe_port *pdata)
 	return pdata->phy_if.phy_impl.an_outcome(pdata);
 }
 
-static void axgbe_phy_status_result(struct axgbe_port *pdata)
+static bool axgbe_phy_status_result(struct axgbe_port *pdata)
 {
 	enum axgbe_mode mode;
 
@@ -1065,8 +1065,13 @@ static void axgbe_phy_status_result(struct axgbe_port *pdata)
 
 	pdata->phy.duplex = DUPLEX_FULL;
 
-	if (axgbe_set_mode(pdata, mode) && pdata->an_again)
+	if (!axgbe_set_mode(pdata, mode))
+		return false;
+
+	if (pdata->an_again)
 		axgbe_phy_reconfig_aneg(pdata);
+
+	return true;
 }
 
 static int autoneg_time_out(unsigned long autoneg_start_time)
@@ -1133,7 +1138,10 @@ static void axgbe_phy_status(struct axgbe_port *pdata)
 				return;
 			}
 		}
-		axgbe_phy_status_result(pdata);
+
+		if (axgbe_phy_status_result(pdata))
+			return;
+
 		if (rte_bit_relaxed_get32(AXGBE_LINK_INIT, &pdata->dev_state))
 			rte_bit_relaxed_clear32(AXGBE_LINK_INIT,
 						&pdata->dev_state);
-- 
2.34.1

RE: [PATCH v2 23/25] net/axgbe: fix the false linkup in axgbe PHY status

[Sebastian, Selwin]

[AMD Official Use Only - AMD Internal Distribution Only]

Acked-by: Selwin Sebastian<selwin.sebastian@amd.com>

-----Original Message-----
From: Ande, Venkat Kumar <VenkatKumar.Ande@amd.com>
Sent: Tuesday, May 7, 2024 6:13 PM
To: dev@dpdk.org
Cc: Sebastian, Selwin <Selwin.Sebastian@amd.com>; Ande, Venkat Kumar <VenkatKumar.Ande@amd.com>; stable@dpdk.org
Subject: [PATCH v2 23/25] net/axgbe: fix the false linkup in axgbe PHY status

In the event of a change in AXGBE mode, the current auto-negotiation needs to be reset and the AN cycle needs to be re-triggered. However, the current code ignores the return value of axgbe_set_mode(), leading to false information as the link is declared without checking the status register.

Fix this by propagating the mode switch status information to axgbe_phy_status().

Fixes: 102b6ec3d5c3 ("net/axgbe: support auto-negotiation for 1Gbps")
Cc: stable@dpdk.org

Signed-off-by: Venkat Kumar Ande <venkatkumar.ande@amd.com>
---
 drivers/net/axgbe/axgbe_mdio.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/net/axgbe/axgbe_mdio.c b/drivers/net/axgbe/axgbe_mdio.c index 9fe30e83bc..130a67479e 100644
--- a/drivers/net/axgbe/axgbe_mdio.c
+++ b/drivers/net/axgbe/axgbe_mdio.c
@@ -1028,7 +1028,7 @@ static enum axgbe_mode axgbe_phy_status_aneg(struct axgbe_port *pdata)
        return pdata->phy_if.phy_impl.an_outcome(pdata);
 }

-static void axgbe_phy_status_result(struct axgbe_port *pdata)
+static bool axgbe_phy_status_result(struct axgbe_port *pdata)
 {
        enum axgbe_mode mode;

@@ -1065,8 +1065,13 @@ static void axgbe_phy_status_result(struct axgbe_port *pdata)

        pdata->phy.duplex = DUPLEX_FULL;

-       if (axgbe_set_mode(pdata, mode) && pdata->an_again)
+       if (!axgbe_set_mode(pdata, mode))
+               return false;
+
+       if (pdata->an_again)
                axgbe_phy_reconfig_aneg(pdata);
+
+       return true;
 }

 static int autoneg_time_out(unsigned long autoneg_start_time) @@ -1133,7 +1138,10 @@ static void axgbe_phy_status(struct axgbe_port *pdata)
                                return;
                        }
                }
-               axgbe_phy_status_result(pdata);
+
+               if (axgbe_phy_status_result(pdata))
+                       return;
+
                if (rte_bit_relaxed_get32(AXGBE_LINK_INIT, &pdata->dev_state))
                        rte_bit_relaxed_clear32(AXGBE_LINK_INIT,
                                                &pdata->dev_state);
--
2.34.1

Re: [PATCH v2 23/25] net/axgbe: fix the false linkup in axgbe PHY status

[Ferruh Yigit]

On 5/7/2024 1:43 PM, Venkat Kumar Ande wrote:
> In the event of a change in AXGBE mode, the current auto-negotiation
> needs to be reset and the AN cycle needs to be re-triggered. However,
> the current code ignores the return value of axgbe_set_mode(), leading to
> false information as the link is declared without checking the status
> register.
> 
> Fix this by propagating the mode switch status information to
> axgbe_phy_status().
> 
> Fixes: 102b6ec3d5c3 ("net/axgbe: support auto-negotiation for 1Gbps")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Venkat Kumar Ande <venkatkumar.ande@amd.com>
>

Should we move this fix too, as done with other patches, at the
beginning of the set?
Otherwise because of previous patches this patch won't apply cleanly to
stable releases.

RE: [PATCH v2 01/25] net/axgbe: fix mdio access for non-zero ports and CL45 PHYs

[Sebastian, Selwin]

[AMD Official Use Only - AMD Internal Distribution Only]

Acked-by: Selwin Sebastian<selwin.sebastian@amd.com>

-----Original Message-----
From: Ande, Venkat Kumar <VenkatKumar.Ande@amd.com>
Sent: Tuesday, May 7, 2024 6:13 PM
To: dev@dpdk.org
Cc: Sebastian, Selwin <Selwin.Sebastian@amd.com>; Ande, Venkat Kumar <VenkatKumar.Ande@amd.com>; stable@dpdk.org
Subject: [PATCH v2 01/25] net/axgbe: fix mdio access for non-zero ports and CL45 PHYs

The XGBE supports performing MDIO operations using an MDIO command request. The driver mistakenly uses the mdio port address as the MDIO command request device address instead of the MDIO command request port address. Additionally, the driver does not properly check for and create a clause 45 MDIO command

Without the fix user will get a incorrect value from PHY device.

Check the supplied MDIO register to determine if the request is a clause
45 operation (MII_ADDR_C45). For a clause 45 operation, extract device address and register number from the supplied MDIO register and use them to set the MDIO command request device address and register number fields.
For a clause 22 operation, the MDIO request device address is set to zero and the MDIO command request register number is set to the supplied MDIO register. In either case, the supplied MDIO port address is used as the MDIO command request port address.

Fixes: 4ac7516b8b39 ("net/axgbe: add phy init and related APIs")
Cc: stable@dpdk.org

Signed-off-by: Venkat Kumar Ande <venkatkumar.ande@amd.com>
---
 drivers/net/axgbe/axgbe_common.h |  2 --
 drivers/net/axgbe/axgbe_dev.c    | 22 ++++++++++++++++------
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/net/axgbe/axgbe_common.h b/drivers/net/axgbe/axgbe_common.h
index a5d11c5832..51532fb34a 100644
--- a/drivers/net/axgbe/axgbe_common.h
+++ b/drivers/net/axgbe/axgbe_common.h
@@ -407,8 +407,6 @@
 #define MAC_MDIOSCAR_PA_WIDTH          5
 #define MAC_MDIOSCAR_RA_INDEX          0
 #define MAC_MDIOSCAR_RA_WIDTH          16
-#define MAC_MDIOSCAR_REG_INDEX         0
-#define MAC_MDIOSCAR_REG_WIDTH         21
 #define MAC_MDIOSCCDR_BUSY_INDEX       22
 #define MAC_MDIOSCCDR_BUSY_WIDTH       1
 #define MAC_MDIOSCCDR_CMD_INDEX                16
diff --git a/drivers/net/axgbe/axgbe_dev.c b/drivers/net/axgbe/axgbe_dev.c index 6a7fddffca..3389954aa6 100644
--- a/drivers/net/axgbe/axgbe_dev.c
+++ b/drivers/net/axgbe/axgbe_dev.c
@@ -63,15 +63,27 @@ static int mdio_complete(struct axgbe_port *pdata)
        return 0;
 }

+static unsigned int axgbe_create_mdio_sca(int port, int reg) {
+       unsigned int mdio_sca, da;
+
+       da = (reg & MII_ADDR_C45) ? reg >> 16 : 0;
+
+       mdio_sca = 0;
+       AXGMAC_SET_BITS(mdio_sca, MAC_MDIOSCAR, RA, reg);
+       AXGMAC_SET_BITS(mdio_sca, MAC_MDIOSCAR, PA, port);
+       AXGMAC_SET_BITS(mdio_sca, MAC_MDIOSCAR, DA, da);
+
+       return mdio_sca;
+}
+
 static int axgbe_write_ext_mii_regs(struct axgbe_port *pdata, int addr,
                                    int reg, u16 val)
 {
        unsigned int mdio_sca, mdio_sccd;
        uint64_t timeout;

-       mdio_sca = 0;
-       AXGMAC_SET_BITS(mdio_sca, MAC_MDIOSCAR, REG, reg);
-       AXGMAC_SET_BITS(mdio_sca, MAC_MDIOSCAR, DA, addr);
+       mdio_sca = axgbe_create_mdio_sca(addr, reg);
        AXGMAC_IOWRITE(pdata, MAC_MDIOSCAR, mdio_sca);

        mdio_sccd = 0;
@@ -97,9 +109,7 @@ static int axgbe_read_ext_mii_regs(struct axgbe_port *pdata, int addr,
        unsigned int mdio_sca, mdio_sccd;
        uint64_t timeout;

-       mdio_sca = 0;
-       AXGMAC_SET_BITS(mdio_sca, MAC_MDIOSCAR, REG, reg);
-       AXGMAC_SET_BITS(mdio_sca, MAC_MDIOSCAR, DA, addr);
+       mdio_sca = axgbe_create_mdio_sca(addr, reg);
        AXGMAC_IOWRITE(pdata, MAC_MDIOSCAR, mdio_sca);

        mdio_sccd = 0;
--
2.34.1

Re: [PATCH v2 01/25] net/axgbe: fix mdio access for non-zero ports and CL45 PHYs

[Ferruh Yigit]

On 5/7/2024 1:42 PM, Venkat Kumar Ande wrote:
> The XGBE supports performing MDIO operations using an MDIO command
> request. The driver mistakenly uses the mdio port address as the
> MDIO command request device address instead of the MDIO command
> request port address. Additionally, the driver does not properly check
> for and create a clause 45 MDIO command
> 
> Without the fix user will get a incorrect value from PHY device.
>

Hi Venkat Kumar,

Thanks for updating the patch order and the updated commit logs, they
have more detailed information now and they are mostly good.

Specific to above note, I will update it as following while merging if
that is OK:
"Without the fix PHY device is not detected and usable."

Thanks,
ferruh