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
next 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