Linux kernel and device drivers for NXP i.MX platforms
 help / color / mirror / Atom feed
From: "Csókás, Bence" <csokas.bence@prolan.hu>
To: Laurent Badel <laurentbadel@eaton.com>,
	Jakub Kicinski <kuba@kernel.org>, <imx@lists.linux.dev>,
	<netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Cc: "Csókás, Bence" <csokas.bence@prolan.hu>,
	"Ahmad Fatoum" <a.fatoum@pengutronix.de>,
	"Simon Horman" <horms@kernel.org>,
	"Michal Swiatkowski" <michal.swiatkowski@linux.intel.com>,
	"Wei Fang" <wei.fang@nxp.com>,
	"Shenwei Wang" <shenwei.wang@nxp.com>,
	"Clark Wang" <xiaoning.wang@nxp.com>,
	"Andrew Lunn" <andrew+netdev@lunn.ch>,
	"David S. Miller" <davem@davemloft.net>,
	"Eric Dumazet" <edumazet@google.com>,
	"Paolo Abeni" <pabeni@redhat.com>
Subject: [PATCH] net: fec: Refactor MAC reset to function
Date: Wed, 22 Jan 2025 17:39:33 +0100	[thread overview]
Message-ID: <20250122163935.213313-2-csokas.bence@prolan.hu> (raw)

The core is reset both in `fec_restart()` (called on link-up) and
`fec_stop()` (going to sleep, driver remove etc.). These two functions
had their separate implementations, which was at first only a register
write and a `udelay()` (and the accompanying block comment). However,
since then we got soft-reset (MAC disable) and Wake-on-LAN support, which
meant that these implementations diverged, often causing bugs. For
instance, as of now, `fec_stop()` does not check for
`FEC_QUIRK_NO_HARD_RESET`, and `fec_restart()` missed the refactor in
commit ff049886671c ("net: fec: Refactor: #define magic constants")
renaming the "magic" constant `1` to `FEC_ECR_RESET`.

To eliminate this bug-source, refactor implementation to a common function.

Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
Fixes: c730ab423bfa ("net: fec: Fix temporary RMII clock reset on link up")
Signed-off-by: Csókás, Bence <csokas.bence@prolan.hu>
---

Notes:
    Recommended options for this patch:
    `--color-moved --color-moved-ws=allow-indentation-change`
    Changes in v2:
    * collect Michal's tag
    * reformat message to 75 cols
    * fix missing `u32 val`

 drivers/net/ethernet/freescale/fec_main.c | 52 +++++++++++------------
 1 file changed, 25 insertions(+), 27 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 68725506a095..520fe638ea00 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -1064,6 +1064,29 @@ static void fec_enet_enable_ring(struct net_device *ndev)
 	}
 }
 
+/* Whack a reset.  We should wait for this.
+ * For i.MX6SX SOC, enet use AXI bus, we use disable MAC
+ * instead of reset MAC itself.
+ */
+static void fec_ctrl_reset(struct fec_enet_private *fep, bool wol)
+{
+	u32 val;
+
+	if (!wol || !(fep->wol_flag & FEC_WOL_FLAG_SLEEP_ON)) {
+		if (fep->quirks & FEC_QUIRK_HAS_MULTI_QUEUES ||
+		    ((fep->quirks & FEC_QUIRK_NO_HARD_RESET) && fep->link)) {
+			writel(0, fep->hwp + FEC_ECNTRL);
+		} else {
+			writel(FEC_ECR_RESET, fep->hwp + FEC_ECNTRL);
+			udelay(10);
+		}
+	} else {
+		val = readl(fep->hwp + FEC_ECNTRL);
+		val |= (FEC_ECR_MAGICEN | FEC_ECR_SLEEP);
+		writel(val, fep->hwp + FEC_ECNTRL);
+	}
+}
+
 /*
  * This function is called to start or restart the FEC during a link
  * change, transmit timeout, or to reconfigure the FEC.  The network
@@ -1080,17 +1103,7 @@ fec_restart(struct net_device *ndev)
 	if (fep->bufdesc_ex)
 		fec_ptp_save_state(fep);
 
-	/* Whack a reset.  We should wait for this.
-	 * For i.MX6SX SOC, enet use AXI bus, we use disable MAC
-	 * instead of reset MAC itself.
-	 */
-	if (fep->quirks & FEC_QUIRK_HAS_MULTI_QUEUES ||
-	    ((fep->quirks & FEC_QUIRK_NO_HARD_RESET) && fep->link)) {
-		writel(0, fep->hwp + FEC_ECNTRL);
-	} else {
-		writel(1, fep->hwp + FEC_ECNTRL);
-		udelay(10);
-	}
+	fec_ctrl_reset(fep, false);
 
 	/*
 	 * enet-mac reset will reset mac address registers too,
@@ -1344,22 +1357,7 @@ fec_stop(struct net_device *ndev)
 	if (fep->bufdesc_ex)
 		fec_ptp_save_state(fep);
 
-	/* Whack a reset.  We should wait for this.
-	 * For i.MX6SX SOC, enet use AXI bus, we use disable MAC
-	 * instead of reset MAC itself.
-	 */
-	if (!(fep->wol_flag & FEC_WOL_FLAG_SLEEP_ON)) {
-		if (fep->quirks & FEC_QUIRK_HAS_MULTI_QUEUES) {
-			writel(0, fep->hwp + FEC_ECNTRL);
-		} else {
-			writel(FEC_ECR_RESET, fep->hwp + FEC_ECNTRL);
-			udelay(10);
-		}
-	} else {
-		val = readl(fep->hwp + FEC_ECNTRL);
-		val |= (FEC_ECR_MAGICEN | FEC_ECR_SLEEP);
-		writel(val, fep->hwp + FEC_ECNTRL);
-	}
+	fec_ctrl_reset(fep, true);
 	writel(fep->phy_speed, fep->hwp + FEC_MII_SPEED);
 	writel(FEC_DEFAULT_IMASK, fep->hwp + FEC_IMASK);
 
-- 
2.48.1



             reply	other threads:[~2025-01-22 16:41 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-22 16:39 Csókás, Bence [this message]
2025-01-22 19:47 ` [PATCH] net: fec: Refactor MAC reset to function Jacob Keller
2025-01-23  1:24 ` Wei Fang
2025-02-04  9:37 ` [PATCH net v3] " Csókás, Bence
2025-02-04 15:45   ` Jakub Kicinski
2025-02-05 13:53     ` Csókás Bence
2025-02-06  1:06       ` Jakub Kicinski
  -- strict thread matches above, loose matches on Subject: below --
2025-02-04  9:36 [PATCH] " Csókás, Bence
2025-02-05 13:28 ` Simon Horman
2025-02-05 13:48   ` Simon Horman
2025-02-06 13:02     ` Csókás Bence
2025-02-06 18:32       ` Simon Horman
2025-01-21 10:38 Csókás, Bence
2025-01-21 14:10 ` Michal Swiatkowski
2025-01-21 14:36 ` Ahmad Fatoum
2025-01-21 15:19 ` Simon Horman
2025-01-21 15:51   ` Csókás Bence
2025-01-21 18:07     ` Jakub Kicinski
2025-01-21 17:28 ` Andrew Lunn
2025-01-22  2:50 ` Wei Fang

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=20250122163935.213313-2-csokas.bence@prolan.hu \
    --to=csokas.bence@prolan.hu \
    --cc=a.fatoum@pengutronix.de \
    --cc=andrew+netdev@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=imx@lists.linux.dev \
    --cc=kuba@kernel.org \
    --cc=laurentbadel@eaton.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=michal.swiatkowski@linux.intel.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=shenwei.wang@nxp.com \
    --cc=wei.fang@nxp.com \
    --cc=xiaoning.wang@nxp.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox