diff mbox series

[v2,net,1/1] net: fec: Fix temporary RMII clock reset on link up

Message ID 20210125100745.5090-2-laurentbadel@eaton.com (mailing list archive)
State Accepted
Delegated to: Netdev Maintainers
Headers show
Series net: fec: Fix temporary RMII clock reset on link up | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present fail Series targets non-next tree, but doesn't contain any Fixes tags
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for net
netdev/subject_prefix success Link
netdev/cc_maintainers success CCed 4 of 4 maintainers
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch warning CHECK: Prefer using the BIT macro
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/header_inline success Link
netdev/stable success Stable not CCed

Commit Message

Badel, Laurent Jan. 25, 2021, 10:07 a.m. UTC
fec_restart() does a hard reset of the MAC module when the link status
changes to up. This temporarily resets the R_CNTRL register which controls
the MII mode of the ENET_OUT clock. In the case of RMII, the clock
frequency momentarily drops from 50MHz to 25MHz until the register is
reconfigured. Some link partners do not tolerate this glitch and
invalidate the link causing failure to establish a stable link when using
PHY polling mode. Since as per IEEE802.11 the criteria for link validity
are PHY-specific, what the partner should tolerate cannot be assumed, so
avoid resetting the MII clock by using software reset instead of hardware
reset when the link is up. This is generally relevant only if the SoC
provides the clock to an external PHY and the PHY is configured for RMII.

Signed-off-by: Laurent Badel <laurentbadel@eaton.com>
---
 drivers/net/ethernet/freescale/fec.h      | 5 +++++
 drivers/net/ethernet/freescale/fec_main.c | 6 ++++--
 2 files changed, 9 insertions(+), 2 deletions(-)

Comments

Jakub Kicinski Jan. 27, 2021, 2:25 a.m. UTC | #1
On Mon, 25 Jan 2021 11:07:45 +0100 Laurent Badel wrote:
> fec_restart() does a hard reset of the MAC module when the link status
> changes to up. This temporarily resets the R_CNTRL register which controls
> the MII mode of the ENET_OUT clock. In the case of RMII, the clock
> frequency momentarily drops from 50MHz to 25MHz until the register is
> reconfigured. Some link partners do not tolerate this glitch and
> invalidate the link causing failure to establish a stable link when using
> PHY polling mode. Since as per IEEE802.11 the criteria for link validity

I think you meant 802.3, fixed that up and applied, thanks!

> are PHY-specific, what the partner should tolerate cannot be assumed, so
> avoid resetting the MII clock by using software reset instead of hardware
> reset when the link is up. This is generally relevant only if the SoC
> provides the clock to an external PHY and the PHY is configured for RMII.
Badel, Laurent Jan. 27, 2021, 10:24 a.m. UTC | #2
> From: Jakub Kicinski <kuba@kernel.org>
> Sent: Wednesday, January 27, 2021 3:26 AM
> > PHY polling mode. Since as per IEEE802.11 the criteria for link
> validity
> 
> I think you meant 802.3, fixed that up and applied, thanks!
> 

You are right, I got confused. 
Thank you for your patience, I am only beginning to understand how things should be, and should not be, done, but hopefully I'll get there someday. 
Best regards,
Laurent


-----------------------------
Eaton Industries Manufacturing GmbH ~ Registered place of business: Route de la Longeraie 7, 1110, Morges, Switzerland 

-----------------------------
diff mbox series

Patch

diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
index c527f4ee1d3a..0602d5d5d2ee 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -462,6 +462,11 @@  struct bufdesc_ex {
  */
 #define FEC_QUIRK_CLEAR_SETUP_MII	(1 << 17)
 
+/* Some link partners do not tolerate the momentary reset of the REF_CLK
+ * frequency when the RNCTL register is cleared by hardware reset.
+ */
+#define FEC_QUIRK_NO_HARD_RESET		(1 << 18)
+
 struct bufdesc_prop {
 	int qid;
 	/* Address of Rx and Tx buffers */
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 04f24c66cf36..0720f36ae384 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -100,7 +100,8 @@  static const struct fec_devinfo fec_imx27_info = {
 static const struct fec_devinfo fec_imx28_info = {
 	.quirks = FEC_QUIRK_ENET_MAC | FEC_QUIRK_SWAP_FRAME |
 		  FEC_QUIRK_SINGLE_MDIO | FEC_QUIRK_HAS_RACC |
-		  FEC_QUIRK_HAS_FRREG | FEC_QUIRK_CLEAR_SETUP_MII,
+		  FEC_QUIRK_HAS_FRREG | FEC_QUIRK_CLEAR_SETUP_MII |
+		  FEC_QUIRK_NO_HARD_RESET,
 };
 
 static const struct fec_devinfo fec_imx6q_info = {
@@ -953,7 +954,8 @@  fec_restart(struct net_device *ndev)
 	 * For i.MX6SX SOC, enet use AXI bus, we use disable MAC
 	 * instead of reset MAC itself.
 	 */
-	if (fep->quirks & FEC_QUIRK_HAS_AVB) {
+	if (fep->quirks & FEC_QUIRK_HAS_AVB ||
+	    ((fep->quirks & FEC_QUIRK_NO_HARD_RESET) && fep->link)) {
 		writel(0, fep->hwp + FEC_ECNTRL);
 	} else {
 		writel(1, fep->hwp + FEC_ECNTRL);