All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Karr <dkarr@vyex.com>
To: netdev@vger.kernel.org
Subject: [RFT PATCH] net: fec: Fix MDIO polled IO
Date: Sun, 9 Aug 2020 18:00:33 -0500	[thread overview]
Message-ID: <813a0ccb-a309-137d-a340-b9b03b9e230c@vyex.com> (raw)

Fixes problematic commit f166f890c8f026a931e1bb80f51561a1d2f41b27

Commit broke Ethernet functionality on i.MX53 with 1 GHz clock due to a
race condition which may not be observed in the unknown CPU(s) implied
by comments to depart from FEC behavior documented by Motorola,
Freescale and NXP.

During driver initialization the stated intent was to suppress the
potential generation of a MII interrupt resulting from a write to
MSCR, but for devices which behave as documented, an interrupt is
caused by any write to MMFR regardless of value. Testing confirms
the i.MX53 behaves as documented.

The subsequent attempt to clear any pending MII interrupt generated by
either the MMFR or MSCR write is a race condition dependent upon
CPU vs. MDC clock speed which permits the interrupt to occur after the
write to EIR.

Prior to the polled IO patch, regardless of what caused an MII
interrupt, the fec_enet_mdio_read/write functions relied upon the
interrupt service routine to clear the EIR MII bit and thus the
critical region between ISR exit and fec_enet_mdio_read/write entry
was small by comparison.

Together, this resulted in the subsequent read of PHYID during PHY
probing to return before the mdio bus transaction was complete
causing invalid data to be read from MMFR which in turn resulted in
improper identification of PHY devices on the bus.

Fix by eliminating dependency on other functions to clear the EIR MII
bit and shrink the critical region by clearing the bit immediately
prior to MMFR write within fec_enet_mdio_read/write functions.

Remove use of undocumented behavior and replace unconditional reset of
MII EIR bit with fec_enet_mdio_wait such that NXP can identify whether
regressions noted in 21615efa6a69891fa287bade979d56dd68b09878 and/or
0a699302be5986307b3dcf84ac7a0dd30f9e9305 are due to an intentional write
to MMFR with MSCR equal to zero prior to driver initialization, or the
result of unpublished or unrealized FEC errata.

Reuse FEC_MII_TIMEOUT #define in fec_enet_mdio_wait().

Signed-off-by: Dave Karr <dkarr@vyex.com>
---
 drivers/net/ethernet/freescale/fec_main.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 9934421814b4..a56169a3b0a9 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -1792,7 +1792,7 @@ static int fec_enet_mdio_wait(struct fec_enet_private *fep)
 	int ret;
 
 	ret = readl_poll_timeout_atomic(fep->hwp + FEC_IEVENT, ievent,
-					ievent & FEC_ENET_MII, 2, 30000);
+					ievent & FEC_ENET_MII, 2, FEC_MII_TIMEOUT);
 
 	if (!ret)
 		writel(FEC_ENET_MII, fep->hwp + FEC_IEVENT);
@@ -1816,6 +1816,7 @@ static int fec_enet_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
 
 		/* write address */
 		frame_addr = (regnum >> 16);
+		writel(FEC_ENET_MII, fep->hwp + FEC_IEVENT);
 		writel(frame_start | FEC_MMFR_OP_ADDR_WRITE |
 		       FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(frame_addr) |
 		       FEC_MMFR_TA | (regnum & 0xFFFF),
@@ -1838,6 +1839,7 @@ static int fec_enet_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
 	}
 
 	/* start a read op */
+	writel(FEC_ENET_MII, fep->hwp + FEC_IEVENT);
 	writel(frame_start | frame_op |
 		FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(frame_addr) |
 		FEC_MMFR_TA, fep->hwp + FEC_MII_DATA);
@@ -1877,6 +1879,7 @@ static int fec_enet_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
 
 		/* write address */
 		frame_addr = (regnum >> 16);
+		writel(FEC_ENET_MII, fep->hwp + FEC_IEVENT);
 		writel(frame_start | FEC_MMFR_OP_ADDR_WRITE |
 		       FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(frame_addr) |
 		       FEC_MMFR_TA | (regnum & 0xFFFF),
@@ -1895,6 +1898,7 @@ static int fec_enet_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
 	}
 
 	/* start a write op */
+	writel(FEC_ENET_MII, fep->hwp + FEC_IEVENT);
 	writel(frame_start | FEC_MMFR_OP_WRITE |
 		FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(frame_addr) |
 		FEC_MMFR_TA | FEC_MMFR_DATA(value),
@@ -2114,20 +2118,14 @@ static int fec_enet_mii_init(struct platform_device *pdev)
 	if (suppress_preamble)
 		fep->phy_speed |= BIT(7);
 
-	/* Clear MMFR to avoid to generate MII event by writing MSCR.
-	 * MII event generation condition:
-	 * - writing MSCR:
-	 *	- mmfr[31:0]_not_zero & mscr[7:0]_is_zero &
-	 *	  mscr_reg_data_in[7:0] != 0
-	 * - writing MMFR:
-	 *	- mscr[7:0]_not_zero
-	 */
-	writel(0, fep->hwp + FEC_MII_DATA);
+	/* start with mii interrupt flag clear */
+	writel(FEC_ENET_MII, fep->hwp + FEC_IEVENT);
 
 	writel(fep->phy_speed, fep->hwp + FEC_MII_SPEED);
 
 	/* Clear any pending transaction complete indication */
-	writel(FEC_ENET_MII, fep->hwp + FEC_IEVENT);
+	if (!(fec_enet_mdio_wait(fep)))
+		netdev_dbg(fep->netdev, "MSCR write during init caused mii interrupt\n");
 
 	fep->mii_bus = mdiobus_alloc();
 	if (fep->mii_bus == NULL) {
-- 
2.26.2


             reply	other threads:[~2020-08-09 23:47 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-09 23:00 Dave Karr [this message]
2020-08-15 21:00 ` [RFT] net: fec: Fix MDIO polled IO Clemens Gruber

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=813a0ccb-a309-137d-a340-b9b03b9e230c@vyex.com \
    --to=dkarr@vyex.com \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.