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

Message ID 20240507124305.2318-23-venkatkumar.ande@amd.com (mailing list archive)
State Changes Requested
Delegated to: Ferruh Yigit
Headers
Series [v2,01/25] net/axgbe: fix mdio access for non-zero ports and CL45 PHYs |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Venkat Kumar Ande May 7, 2024, 12:43 p.m. UTC
  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(-)
  

Comments

Sebastian, Selwin May 20, 2024, 10:43 a.m. UTC | #1
[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
  
Ferruh Yigit May 20, 2024, 11:25 a.m. UTC | #2
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.
  

Patch

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);