From: "Russell King (Oracle)" <rmk+kernel@armlinux•org.uk>
To: Jose Abreu <Jose.Abreu@synopsys•com>
Cc: Andrew Lunn <andrew@lunn•ch>,
"David S. Miller" <davem@davemloft•net>,
Eric Dumazet <edumazet@google•com>,
Heiner Kallweit <hkallweit1@gmail•com>,
Jakub Kicinski <kuba@kernel•org>,
netdev@vger•kernel.org, Paolo Abeni <pabeni@redhat•com>
Subject: [PATCH RFC net-next 9/9] net: pcs: xpcs: avoid reading STAT1 more than once
Date: Fri, 12 May 2023 18:27:45 +0100 [thread overview]
Message-ID: <E1pxWYT-002Qsg-Rg@rmk-PC.armlinux.org.uk> (raw)
In-Reply-To: <ZF52z7PqH2HLrWEU@shell.armlinux.org.uk>
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>
---
drivers/net/pcs/pcs-xpcs.c | 91 +++++++++++++++++++++-----------------
1 file changed, 50 insertions(+), 41 deletions(-)
diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
index c5fe944f48dd..a58d9d079eca 100644
--- 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 stat1;
+ }
+
/* Link needs to be read first ... */
- state->link = xpcs_read_link_c73(xpcs) > 0 ? 1 : 0;
+ state->link = !!(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);
}
--
2.30.2
next prev parent reply other threads:[~2023-05-12 17:27 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-12 17:26 [PATCH RFC net-next 0/9] net: pcs: xpcs: cleanups for clause 73 support Russell King (Oracle)
2023-05-12 17:27 ` [PATCH RFC net-next 1/9] net: mdio: add clause 73 to ethtool conversion helper Russell King (Oracle)
2023-05-12 23:47 ` Andrew Lunn
2023-05-12 17:27 ` [PATCH RFC net-next 2/9] net: phylink: remove duplicated linkmode pause resolution Russell King (Oracle)
2023-05-12 23:52 ` Andrew Lunn
2023-05-12 17:27 ` [PATCH RFC net-next 3/9] net: phylink: add function to resolve clause 73 negotiation Russell King (Oracle)
2023-05-12 23:57 ` Andrew Lunn
2023-05-13 9:24 ` Russell King (Oracle)
2023-05-13 13:55 ` Andrew Lunn
2023-05-12 17:27 ` [PATCH RFC net-next 4/9] net: pcs: xpcs: clean up reading clause 73 link partner advertisement Russell King (Oracle)
2023-05-13 0:01 ` Andrew Lunn
2023-05-12 17:27 ` [PATCH RFC net-next 5/9] net: pcs: xpcs: use mii_c73_to_linkmode() helper Russell King (Oracle)
2023-05-13 0:05 ` Andrew Lunn
2023-05-12 17:27 ` [PATCH RFC net-next 6/9] net: pcs: xpcs: correct lp_advertising contents Russell King (Oracle)
2023-05-13 17:39 ` Andrew Lunn
2023-05-12 17:27 ` [PATCH RFC net-next 7/9] net: pcs: xpcs: correct pause resolution Russell King (Oracle)
2023-05-13 17:47 ` Andrew Lunn
2023-05-13 18:13 ` Russell King (Oracle)
2023-05-13 18:17 ` Andrew Lunn
2023-05-12 17:27 ` [PATCH RFC net-next 8/9] net: pcs: xpcs: use phylink_resolve_c73() helper Russell King (Oracle)
2023-05-12 19:38 ` Simon Horman
2023-05-12 17:27 ` Russell King (Oracle) [this message]
2023-05-12 19:36 ` [PATCH RFC net-next 9/9] net: pcs: xpcs: avoid reading STAT1 more than once Simon Horman
-- strict thread matches above, loose matches on Subject: below --
2023-05-17 14:11 [PATCH RFC v2 net-next 0/9] net: pcs: xpcs: cleanups for clause 73 support Russell King (Oracle)
2023-05-17 14:12 ` [PATCH RFC net-next 9/9] net: pcs: xpcs: avoid reading STAT1 more than once Russell King (Oracle)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=E1pxWYT-002Qsg-Rg@rmk-PC.armlinux.org.uk \
--to=rmk+kernel@armlinux$(echo .)org.uk \
--cc=Jose.Abreu@synopsys$(echo .)com \
--cc=andrew@lunn$(echo .)ch \
--cc=davem@davemloft$(echo .)net \
--cc=edumazet@google$(echo .)com \
--cc=hkallweit1@gmail$(echo .)com \
--cc=kuba@kernel$(echo .)org \
--cc=netdev@vger$(echo .)kernel.org \
--cc=pabeni@redhat$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox