net: pcs: xpcs: avoid reading STAT1 more than once

Avoid reading the STAT1 registers more than once while getting the PCS
state, as this register contains latching-low bits that are lost after
the first read.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
index 7c45887..736776e 100644 (file)
--- a/drivers/net/pcs/pcs-xpcs.c
+++ b/drivers/net/pcs/pcs-xpcs.c
@@ -271,15 +271,12 @@ static int xpcs_soft_reset(struct dw_xpcs *xpcs,
 })
 
 static int xpcs_read_fault_c73(struct dw_xpcs *xpcs,
-                              struct phylink_link_state *state)
+                              struct phylink_link_state *state,
+                              u16 pcs_stat1)
 {
        int ret;
 
-       ret = xpcs_read(xpcs, MDIO_MMD_PCS, MDIO_STAT1);
-       if (ret < 0)
-               return ret;
-
-       if (ret & MDIO_STAT1_FAULT) {
+       if (pcs_stat1 & MDIO_STAT1_FAULT) {
                xpcs_warn(xpcs, state, "Link fault condition detected!\n");
                return -EFAULT;
        }
@@ -321,21 +318,6 @@ static int xpcs_read_fault_c73(struct dw_xpcs *xpcs,
        return 0;
 }
 
-static int xpcs_read_link_c73(struct dw_xpcs *xpcs)
-{
-       bool link = true;
-       int ret;
-
-       ret = xpcs_read(xpcs, MDIO_MMD_PCS, MDIO_STAT1);
-       if (ret < 0)
-               return ret;
-
-       if (!(ret & MDIO_STAT1_LSTATUS))
-               link = false;
-
-       return link;
-}
-
 static void xpcs_config_usxgmii(struct dw_xpcs *xpcs, int speed)
 {
        int ret, speed_sel;
@@ -462,15 +444,11 @@ static int xpcs_config_aneg_c73(struct dw_xpcs *xpcs,
 
 static int xpcs_aneg_done_c73(struct dw_xpcs *xpcs,
                              struct phylink_link_state *state,
-                             const struct xpcs_compat *compat)
+                             const struct xpcs_compat *compat, u16 an_stat1)
 {
        int ret;
 
-       ret = xpcs_read(xpcs, MDIO_MMD_AN, MDIO_STAT1);
-       if (ret < 0)
-               return ret;
-
-       if (ret & MDIO_AN_STAT1_COMPLETE) {
+       if (an_stat1 & MDIO_AN_STAT1_COMPLETE) {
                ret = xpcs_read(xpcs, MDIO_MMD_AN, MDIO_AN_LPA);
                if (ret < 0)
                        return ret;
@@ -488,16 +466,12 @@ static int xpcs_aneg_done_c73(struct dw_xpcs *xpcs,
 }
 
 static int xpcs_read_lpa_c73(struct dw_xpcs *xpcs,
-                            struct phylink_link_state *state)
+                            struct phylink_link_state *state, u16 an_stat1)
 {
        u16 lpa[3];
        int i, ret;
 
-       ret = xpcs_read(xpcs, MDIO_MMD_AN, MDIO_STAT1);
-       if (ret < 0)
-               return ret;
-
-       if (!(ret & MDIO_AN_STAT1_LPABLE)) {
+       if (!(an_stat1 & MDIO_AN_STAT1_LPABLE)) {
                phylink_clear(state->lp_advertising, Autoneg);
                return 0;
        }
@@ -880,13 +854,25 @@ static int xpcs_get_state_c73(struct dw_xpcs *xpcs,
                              const struct xpcs_compat *compat)
 {
        bool an_enabled;
+       int pcs_stat1;
+       int an_stat1;
        int ret;
 
+       /* The link status bit is latching-low, so it is important to
+        * avoid unnecessary re-reads of this register to avoid missing
+        * a link-down event.
+        */
+       pcs_stat1 = xpcs_read(xpcs, MDIO_MMD_PCS, MDIO_STAT1);
+       if (pcs_stat1 < 0) {
+               state->link = false;
+               return pcs_stat1;
+       }
+
        /* Link needs to be read first ... */
-       state->link = xpcs_read_link_c73(xpcs) > 0 ? 1 : 0;
+       state->link = !!(pcs_stat1 & MDIO_STAT1_LSTATUS);
 
        /* ... and then we check the faults. */
-       ret = xpcs_read_fault_c73(xpcs, state);
+       ret = xpcs_read_fault_c73(xpcs, state, pcs_stat1);
        if (ret) {
                ret = xpcs_soft_reset(xpcs, compat);
                if (ret)
@@ -897,15 +883,38 @@ static int xpcs_get_state_c73(struct dw_xpcs *xpcs,
                return xpcs_do_config(xpcs, state->interface, MLO_AN_INBAND, NULL);
        }
 
+       /* There is no point doing anything else if the link is down. */
+       if (!state->link)
+               return 0;
+
        an_enabled = linkmode_test_bit(ETHTOOL_LINK_MODE_Autoneg_BIT,
                                       state->advertising);
-       if (an_enabled && xpcs_aneg_done_c73(xpcs, state, compat)) {
-               state->an_complete = true;
-               xpcs_read_lpa_c73(xpcs, state);
+       if (an_enabled) {
+               /* The link status bit is latching-low, so it is important to
+                * avoid unnecessary re-reads of this register to avoid missing
+                * a link-down event.
+                */
+               an_stat1 = xpcs_read(xpcs, MDIO_MMD_AN, MDIO_STAT1);
+               if (an_stat1 < 0) {
+                       state->link = false;
+                       return an_stat1;
+               }
+
+               state->an_complete = xpcs_aneg_done_c73(xpcs, state, compat,
+                                                       an_stat1);
+               if (!state->an_complete) {
+                       state->link = false;
+                       return 0;
+               }
+
+               ret = xpcs_read_lpa_c73(xpcs, state, an_stat1);
+               if (ret < 0) {
+                       state->link = false;
+                       return ret;
+               }
+
                phylink_resolve_c73(state);
-       } else if (an_enabled) {
-               state->link = 0;
-       } else if (state->link) {
+       } else {
                xpcs_resolve_pma(xpcs, state);
        }