public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH phy-next 0/3] Lynx 28G: better init(), exit(), power_on(), power_off()
@ 2026-03-21  1:14 Vladimir Oltean
  2026-03-21  1:14 ` [PATCH phy-next 1/3] phy: lynx-28g: use timeouts when waiting for lane halt and reset Vladimir Oltean
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Vladimir Oltean @ 2026-03-21  1:14 UTC (permalink / raw)
  To: linux-phy
  Cc: netdev, Ioana Ciornei, Vinod Koul, Neil Armstrong, Josua Mayer,
	linux-kernel

This is a set of 3 improvements to the 28G Lynx SerDes driver as found
on NXP Layerscape:
- avoid kernel hangs if lane resets/halts fail due to other bugs
- actually have phy_power_down() cut power from lanes, not just halt them
- allow consumers to call phy_exit(), to balance the phy->init_count

Especially change 3 will allow further development of the dpaa2-eth
consumer. To permit phy_exit() and other patches in net-next to be
submitted in this development cycle without functionally breaking
networking, please apply this change on top of v7.0-rc1 and provide
it as a stable tag to be pulled in netdev.

Vladimir Oltean (3):
  phy: lynx-28g: use timeouts when waiting for lane halt and reset
  phy: lynx-28g: truly power the lanes up or down
  phy: lynx-28g: implement phy_exit() operation

 drivers/phy/freescale/phy-fsl-lynx-28g.c | 190 +++++++++++++++++++----
 1 file changed, 157 insertions(+), 33 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH phy-next 1/3] phy: lynx-28g: use timeouts when waiting for lane halt and reset
  2026-03-21  1:14 [PATCH phy-next 0/3] Lynx 28G: better init(), exit(), power_on(), power_off() Vladimir Oltean
@ 2026-03-21  1:14 ` Vladimir Oltean
  2026-03-21 15:48   ` Andrew Lunn
  2026-03-21  1:14 ` [PATCH phy-next 2/3] phy: lynx-28g: truly power the lanes up or down Vladimir Oltean
  2026-03-21  1:14 ` [PATCH phy-next 3/3] phy: lynx-28g: implement phy_exit() operation Vladimir Oltean
  2 siblings, 1 reply; 5+ messages in thread
From: Vladimir Oltean @ 2026-03-21  1:14 UTC (permalink / raw)
  To: linux-phy
  Cc: netdev, Ioana Ciornei, Vinod Koul, Neil Armstrong, Josua Mayer,
	linux-kernel

There are various circumstances in which a lane halt, or a lane reset,
will fail to complete. If this happens, it will hang the kernel, which
only implements a busy loop with no timeout.

The circumstances in which this will happen are all bugs in nature:
- if we try to power off a powered off lane
- if we try to power off a lane that uses a PLL locked onto the wrong
  refclk frequency (wrong RCW, but SoC boots anyway)

Actually, unbounded loops in the kernel are a bad practice, so let's use
read_poll_timeout() with a custom function that reads both LNaTRSTCTL
(lane transmit control register) and LNaRRSTCTL (lane receive control
register) and returns true when the request is done in both directions.

The HLT_REQ bit has to clear, whereas the RST_DONE bit has to get set.

Any time such an error happens, it is catastrophic and there is no point
in trying to propagate it to our callers:
- if lynx_28g_set_mode() -> lynx_28g_power_on() times out, we have
  already reconfigured the lane, but returning an error would tell the
  caller that we didn't
- if lynx_28g_power_off() times out, again not much for the consumer to
  do to help get out of this situation - the phy_power_off() call is
  probably made from a context that the consumer can't cancel, or it is
  making it to return to a known state from a previous failure.

So just print an error if timeouts happen and let the driver control
flow continue. The entire point is just to not let the kernel freeze.

Suggested-by: Josua Mayer <josua@solid-run.com>
Link: https://lore.kernel.org/lkml/d0c8bbf8-a0c5-469f-a148-de2235948c0f@solid-run.com/
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Previously submitted as part of larger set:
https://lore.kernel.org/linux-phy/20260114152111.625350-7-vladimir.oltean@nxp.com/

Changes:
- Stop propagating the read_poll_timeout() errors to callers
- Adjust commit message to explain that decision
---
 drivers/phy/freescale/phy-fsl-lynx-28g.c | 72 ++++++++++++++++++------
 1 file changed, 56 insertions(+), 16 deletions(-)

diff --git a/drivers/phy/freescale/phy-fsl-lynx-28g.c b/drivers/phy/freescale/phy-fsl-lynx-28g.c
index 2b0fd95ba62f..3debf4131e0f 100644
--- a/drivers/phy/freescale/phy-fsl-lynx-28g.c
+++ b/drivers/phy/freescale/phy-fsl-lynx-28g.c
@@ -249,6 +249,12 @@
 
 #define CR(x)					((x) * 4)
 
+#define LYNX_28G_LANE_HALT_SLEEP_US		100
+#define LYNX_28G_LANE_HALT_TIMEOUT_US		1000000
+
+#define LYNX_28G_LANE_RESET_SLEEP_US		100
+#define LYNX_28G_LANE_RESET_TIMEOUT_US		1000000
+
 enum lynx_28g_eq_type {
 	EQ_TYPE_NO_EQ = 0,
 	EQ_TYPE_2TAP = 1,
@@ -600,10 +606,29 @@ static void lynx_28g_lane_set_pll(struct lynx_28g_lane *lane,
 	}
 }
 
+static bool lynx_28g_lane_halt_done(struct lynx_28g_lane *lane)
+{
+	u32 trstctl = lynx_28g_lane_read(lane, LNaTRSTCTL);
+	u32 rrstctl = lynx_28g_lane_read(lane, LNaRRSTCTL);
+
+	return !(trstctl & LNaTRSTCTL_HLT_REQ) &&
+	       !(rrstctl & LNaRRSTCTL_HLT_REQ);
+}
+
+static bool lynx_28g_lane_reset_done(struct lynx_28g_lane *lane)
+{
+	u32 trstctl = lynx_28g_lane_read(lane, LNaTRSTCTL);
+	u32 rrstctl = lynx_28g_lane_read(lane, LNaRRSTCTL);
+
+	return (trstctl & LNaTRSTCTL_RST_DONE) &&
+	       (rrstctl & LNaRRSTCTL_RST_DONE);
+}
+
 static int lynx_28g_power_off(struct phy *phy)
 {
 	struct lynx_28g_lane *lane = phy_get_drvdata(phy);
-	u32 trstctl, rrstctl;
+	bool done;
+	int err;
 
 	if (!lane->powered_up)
 		return 0;
@@ -615,11 +640,14 @@ static int lynx_28g_power_off(struct phy *phy)
 			  LNaRRSTCTL_HLT_REQ);
 
 	/* Wait until the halting process is complete */
-	do {
-		trstctl = lynx_28g_lane_read(lane, LNaTRSTCTL);
-		rrstctl = lynx_28g_lane_read(lane, LNaRRSTCTL);
-	} while ((trstctl & LNaTRSTCTL_HLT_REQ) ||
-		 (rrstctl & LNaRRSTCTL_HLT_REQ));
+	err = read_poll_timeout(lynx_28g_lane_halt_done, done, done,
+				LYNX_28G_LANE_HALT_SLEEP_US,
+				LYNX_28G_LANE_HALT_TIMEOUT_US,
+				false, lane);
+	if (err) {
+		dev_err(&phy->dev, "Lane %c halt failed: %pe\n",
+			'A' + lane->id, ERR_PTR(err));
+	}
 
 	lane->powered_up = false;
 
@@ -629,7 +657,8 @@ static int lynx_28g_power_off(struct phy *phy)
 static int lynx_28g_power_on(struct phy *phy)
 {
 	struct lynx_28g_lane *lane = phy_get_drvdata(phy);
-	u32 trstctl, rrstctl;
+	bool done;
+	int err;
 
 	if (lane->powered_up)
 		return 0;
@@ -641,11 +670,14 @@ static int lynx_28g_power_on(struct phy *phy)
 			  LNaRRSTCTL_RST_REQ);
 
 	/* Wait until the reset sequence is completed */
-	do {
-		trstctl = lynx_28g_lane_read(lane, LNaTRSTCTL);
-		rrstctl = lynx_28g_lane_read(lane, LNaRRSTCTL);
-	} while (!(trstctl & LNaTRSTCTL_RST_DONE) ||
-		 !(rrstctl & LNaRRSTCTL_RST_DONE));
+	err = read_poll_timeout(lynx_28g_lane_reset_done, done, done,
+				LYNX_28G_LANE_RESET_SLEEP_US,
+				LYNX_28G_LANE_RESET_TIMEOUT_US,
+				false, lane);
+	if (err) {
+		dev_err(&phy->dev, "Lane %c reset failed: %pe\n",
+			'A' + lane->id, ERR_PTR(err));
+	}
 
 	lane->powered_up = true;
 
@@ -1065,7 +1097,7 @@ static void lynx_28g_cdr_lock_check(struct work_struct *work)
 	struct lynx_28g_priv *priv = work_to_lynx(work);
 	struct lynx_28g_lane *lane;
 	u32 rrstctl;
-	int i;
+	int err, i;
 
 	for (i = 0; i < LYNX_28G_NUM_LANE; i++) {
 		lane = &priv->lane[i];
@@ -1081,9 +1113,17 @@ static void lynx_28g_cdr_lock_check(struct work_struct *work)
 		if (!(rrstctl & LNaRRSTCTL_CDR_LOCK)) {
 			lynx_28g_lane_rmw(lane, LNaRRSTCTL, LNaRRSTCTL_RST_REQ,
 					  LNaRRSTCTL_RST_REQ);
-			do {
-				rrstctl = lynx_28g_lane_read(lane, LNaRRSTCTL);
-			} while (!(rrstctl & LNaRRSTCTL_RST_DONE));
+
+			err = read_poll_timeout(lynx_28g_lane_read, rrstctl,
+						!!(rrstctl & LNaRRSTCTL_RST_DONE),
+						LYNX_28G_LANE_RESET_SLEEP_US,
+						LYNX_28G_LANE_RESET_TIMEOUT_US,
+						false, lane, LNaRRSTCTL);
+			if (err) {
+				dev_warn_once(&lane->phy->dev,
+					      "Lane %c receiver reset failed: %pe\n",
+					      'A' + lane->id, ERR_PTR(err));
+			}
 		}
 
 		mutex_unlock(&lane->phy->mutex);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH phy-next 2/3] phy: lynx-28g: truly power the lanes up or down
  2026-03-21  1:14 [PATCH phy-next 0/3] Lynx 28G: better init(), exit(), power_on(), power_off() Vladimir Oltean
  2026-03-21  1:14 ` [PATCH phy-next 1/3] phy: lynx-28g: use timeouts when waiting for lane halt and reset Vladimir Oltean
@ 2026-03-21  1:14 ` Vladimir Oltean
  2026-03-21  1:14 ` [PATCH phy-next 3/3] phy: lynx-28g: implement phy_exit() operation Vladimir Oltean
  2 siblings, 0 replies; 5+ messages in thread
From: Vladimir Oltean @ 2026-03-21  1:14 UTC (permalink / raw)
  To: linux-phy
  Cc: netdev, Ioana Ciornei, Vinod Koul, Neil Armstrong, Josua Mayer,
	linux-kernel

The current procedure for power_off() and power_on() is the same as the
one used for major lane reconfiguration, aka halting. But this is
incorrect.

One would expect that a powered off lane causes the CDR (clock and data
recovery) loop of the link partner to lose lock onto its RX stream
(which suggests there are no longer any bit transitions => the channel
is inactive). However, it can be observed that this does not take place
(the CDR lock is still there), which means that a halted lane is not
powered off.

Implement the procedure mentioned in the block guide for powering down
a lane, and then back on. This means:

- lynx_28g_power_off() currently emits a HLT_REQ and waits for it to
  clear. Rename it to lynx_28g_lane_halt() and keep using it for
  lynx_28g_set_mode().
- lynx_28g_power_on() currently emits a RST_REQ and waits for it to
  clear. This is the reset procedure - rename it to
  lynx_28g_lane_reset(), and keep using it for lynx_28g_set_mode().

The "real" lynx_28g_power_off() should emit a STP_REQ and wait for it to
clear. And the "real" lynx_28g_power_on() should clear the DIS bit and
then perform a lane reset. Hook these methods to the phy_ops ::
power_off() and power_on() instead.

As opposed to lynx_28g_power_off() which is also directly hooked to the
PHY API, lynx_28g_lane_halt() isn't; just to lynx_28g_set_mode(), and
the call is being made in a state where we haven't yet made any change
to the lane. So it does make some limited amount of sense to code this
up such that if it fails, we make an attempt to reset the lane anyway
and let the consumer know that phy_set_mode_ext() failed.

Sort the field definitions in LNaTRSTCTL and LNaRRSTCTL in descending
order, and add new definitions for STP_REQ and DIS, previously unused.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Previously submitted as part of larger set:
https://lore.kernel.org/linux-phy/20260114152111.625350-8-vladimir.oltean@nxp.com/

Changes:
- stop propagating read_poll_timeout() error code to the new
  lynx_28g_power_off() implementation, for the same reason as the
  previous patch.
- do propagate the read_poll_timeout() error code to
  lynx_28g_lane_halt(), and make an attempt to reset the lane.
- more detailed explanation in commit message
---
 drivers/phy/freescale/phy-fsl-lynx-28g.c | 103 +++++++++++++++++++----
 1 file changed, 85 insertions(+), 18 deletions(-)

diff --git a/drivers/phy/freescale/phy-fsl-lynx-28g.c b/drivers/phy/freescale/phy-fsl-lynx-28g.c
index 3debf4131e0f..b056951506dc 100644
--- a/drivers/phy/freescale/phy-fsl-lynx-28g.c
+++ b/drivers/phy/freescale/phy-fsl-lynx-28g.c
@@ -70,9 +70,11 @@
 
 /* Lane a Tx Reset Control Register */
 #define LNaTRSTCTL(lane)			(0x800 + (lane) * 0x100 + 0x20)
-#define LNaTRSTCTL_HLT_REQ			BIT(27)
-#define LNaTRSTCTL_RST_DONE			BIT(30)
 #define LNaTRSTCTL_RST_REQ			BIT(31)
+#define LNaTRSTCTL_RST_DONE			BIT(30)
+#define LNaTRSTCTL_HLT_REQ			BIT(27)
+#define LNaTRSTCTL_STP_REQ			BIT(26)
+#define LNaTRSTCTL_DIS				BIT(24)
 
 /* Lane a Tx General Control Register */
 #define LNaTGCR0(lane)				(0x800 + (lane) * 0x100 + 0x24)
@@ -98,9 +100,11 @@
 
 /* Lane a Rx Reset Control Register */
 #define LNaRRSTCTL(lane)			(0x800 + (lane) * 0x100 + 0x40)
-#define LNaRRSTCTL_HLT_REQ			BIT(27)
-#define LNaRRSTCTL_RST_DONE			BIT(30)
 #define LNaRRSTCTL_RST_REQ			BIT(31)
+#define LNaRRSTCTL_RST_DONE			BIT(30)
+#define LNaRRSTCTL_HLT_REQ			BIT(27)
+#define LNaRRSTCTL_STP_REQ			BIT(26)
+#define LNaRRSTCTL_DIS				BIT(24)
 #define LNaRRSTCTL_CDR_LOCK			BIT(12)
 
 /* Lane a Rx General Control Register */
@@ -255,6 +259,9 @@
 #define LYNX_28G_LANE_RESET_SLEEP_US		100
 #define LYNX_28G_LANE_RESET_TIMEOUT_US		1000000
 
+#define LYNX_28G_LANE_STOP_SLEEP_US		100
+#define LYNX_28G_LANE_STOP_TIMEOUT_US		1000000
+
 enum lynx_28g_eq_type {
 	EQ_TYPE_NO_EQ = 0,
 	EQ_TYPE_2TAP = 1,
@@ -615,6 +622,15 @@ static bool lynx_28g_lane_halt_done(struct lynx_28g_lane *lane)
 	       !(rrstctl & LNaRRSTCTL_HLT_REQ);
 }
 
+static bool lynx_28g_lane_stop_done(struct lynx_28g_lane *lane)
+{
+	u32 trstctl = lynx_28g_lane_read(lane, LNaTRSTCTL);
+	u32 rrstctl = lynx_28g_lane_read(lane, LNaRRSTCTL);
+
+	return !(trstctl & LNaTRSTCTL_STP_REQ) &&
+	       !(rrstctl & LNaRRSTCTL_STP_REQ);
+}
+
 static bool lynx_28g_lane_reset_done(struct lynx_28g_lane *lane)
 {
 	u32 trstctl = lynx_28g_lane_read(lane, LNaTRSTCTL);
@@ -624,15 +640,13 @@ static bool lynx_28g_lane_reset_done(struct lynx_28g_lane *lane)
 	       (rrstctl & LNaRRSTCTL_RST_DONE);
 }
 
-static int lynx_28g_power_off(struct phy *phy)
+/* Halting puts the lane in a mode in which it can be reconfigured */
+static int lynx_28g_lane_halt(struct phy *phy)
 {
 	struct lynx_28g_lane *lane = phy_get_drvdata(phy);
 	bool done;
 	int err;
 
-	if (!lane->powered_up)
-		return 0;
-
 	/* Issue a halt request */
 	lynx_28g_lane_rmw(lane, LNaTRSTCTL, LNaTRSTCTL_HLT_REQ,
 			  LNaTRSTCTL_HLT_REQ);
@@ -649,20 +663,15 @@ static int lynx_28g_power_off(struct phy *phy)
 			'A' + lane->id, ERR_PTR(err));
 	}
 
-	lane->powered_up = false;
-
-	return 0;
+	return err;
 }
 
-static int lynx_28g_power_on(struct phy *phy)
+static int lynx_28g_lane_reset(struct phy *phy)
 {
 	struct lynx_28g_lane *lane = phy_get_drvdata(phy);
 	bool done;
 	int err;
 
-	if (lane->powered_up)
-		return 0;
-
 	/* Issue a reset request on the lane */
 	lynx_28g_lane_rmw(lane, LNaTRSTCTL, LNaTRSTCTL_RST_REQ,
 			  LNaTRSTCTL_RST_REQ);
@@ -679,6 +688,61 @@ static int lynx_28g_power_on(struct phy *phy)
 			'A' + lane->id, ERR_PTR(err));
 	}
 
+	return err;
+}
+
+static int lynx_28g_power_off(struct phy *phy)
+{
+	struct lynx_28g_lane *lane = phy_get_drvdata(phy);
+	bool done;
+	int err;
+
+	if (!lane->powered_up)
+		return 0;
+
+	/* Issue a stop request */
+	lynx_28g_lane_rmw(lane, LNaTRSTCTL, LNaTRSTCTL_STP_REQ,
+			  LNaTRSTCTL_STP_REQ);
+	lynx_28g_lane_rmw(lane, LNaRRSTCTL, LNaRRSTCTL_STP_REQ,
+			  LNaRRSTCTL_STP_REQ);
+
+	/* Wait until the stop process is complete */
+	err = read_poll_timeout(lynx_28g_lane_stop_done, done, done,
+				LYNX_28G_LANE_STOP_SLEEP_US,
+				LYNX_28G_LANE_STOP_TIMEOUT_US,
+				false, lane);
+	if (err) {
+		dev_err(&phy->dev, "Lane %c stop failed: %pe\n",
+			'A' + lane->id, ERR_PTR(err));
+	}
+
+	/* Power down the RX and TX portions of the lane */
+	lynx_28g_lane_rmw(lane, LNaRRSTCTL, LNaRRSTCTL_DIS,
+			  LNaRRSTCTL_DIS);
+	lynx_28g_lane_rmw(lane, LNaTRSTCTL, LNaTRSTCTL_DIS,
+			  LNaTRSTCTL_DIS);
+
+	lane->powered_up = false;
+
+	return 0;
+}
+
+static int lynx_28g_power_on(struct phy *phy)
+{
+	struct lynx_28g_lane *lane = phy_get_drvdata(phy);
+	int err;
+
+	if (lane->powered_up)
+		return 0;
+
+	/* Power up the RX and TX portions of the lane */
+	lynx_28g_lane_rmw(lane, LNaRRSTCTL, 0, LNaRRSTCTL_DIS);
+	lynx_28g_lane_rmw(lane, LNaTRSTCTL, 0, LNaTRSTCTL_DIS);
+
+	err = lynx_28g_lane_reset(phy);
+	if (err)
+		return err;
+
 	lane->powered_up = true;
 
 	return 0;
@@ -992,8 +1056,11 @@ static int lynx_28g_set_mode(struct phy *phy, enum phy_mode mode, int submode)
 	/* If the lane is powered up, put the lane into the halt state while
 	 * the reconfiguration is being done.
 	 */
-	if (powered_up)
-		lynx_28g_power_off(phy);
+	if (powered_up) {
+		err = lynx_28g_lane_halt(phy);
+		if (err)
+			goto out;
+	}
 
 	err = lynx_28g_lane_disable_pcvt(lane, lane->mode);
 	if (err)
@@ -1007,7 +1074,7 @@ static int lynx_28g_set_mode(struct phy *phy, enum phy_mode mode, int submode)
 
 out:
 	if (powered_up)
-		lynx_28g_power_on(phy);
+		lynx_28g_lane_reset(phy);
 
 	return err;
 }
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH phy-next 3/3] phy: lynx-28g: implement phy_exit() operation
  2026-03-21  1:14 [PATCH phy-next 0/3] Lynx 28G: better init(), exit(), power_on(), power_off() Vladimir Oltean
  2026-03-21  1:14 ` [PATCH phy-next 1/3] phy: lynx-28g: use timeouts when waiting for lane halt and reset Vladimir Oltean
  2026-03-21  1:14 ` [PATCH phy-next 2/3] phy: lynx-28g: truly power the lanes up or down Vladimir Oltean
@ 2026-03-21  1:14 ` Vladimir Oltean
  2 siblings, 0 replies; 5+ messages in thread
From: Vladimir Oltean @ 2026-03-21  1:14 UTC (permalink / raw)
  To: linux-phy
  Cc: netdev, Ioana Ciornei, Vinod Koul, Neil Armstrong, Josua Mayer,
	linux-kernel

On Layerscape, some Lynx SerDes consumers go through the PHY framework
(we call these 'managed' for the sake of this discussion; they are some
Ethernet lanes), and some consumers don't go through the PHY framework
(we call these 'unmanaged'; they are some unconverted Ethernet lanes,
plus all SATA and PCIe controllers).

A lane is initially unmanaged, and becomes managed when phy_init() is
called on it. Similarly, it becomes unmanaged again when phy_exit() is
called.

Managed lanes are supposed to have power management through
phy_power_on() and phy_power_off().

The lynx-28g SerDes driver, when it probes, probes on all lanes, but
needs to be careful to keep the unmanaged lanes powered on, because
those might be in use by consumers unaware that they need to call
phy_init() and phy_power_on(). This also applies after phy_exit() is
called - no guarantee is made about how the port may be used
afterwards - may be DPDK, may be something else which is unaware of
the PHY framework.

Given this state table:

 State  | Consumer calls phy_exit()? | Provider implements phy_exit()?
 -------+----------------------------+--------------------------------
 (a)    | no                         | no
 (b)    | no                         | yes
 (c)    | yes                        | no
 (d)    | yes                        | yes

we are currently in state (a). This has the problem that when the
consumer fails to probe with -EPROBE_DEFER or is otherwise unbound,
the phy->init_count remains elevated and the lane never returns to the
unmanaged state (temporarily or not) as it should. Furthermore, the
second and subsequent phy_init() consumer calls are never passed on to
the provider driver.

We solve the above problem by implementing phy_exit() in the consumer,
and that moves us to state (c). But this creates the problem that a
balanced set of phy_init() and phy_exit() calls from the consumer will
effectively result in multiple lynx_28g_init() calls as seen by the
SerDes and nothing else - as the optional phy_ops :: exit() is not
implemented. But that actually doesn't work - the 28G Lynx SerDes can't
power down a lane which is already powered down; that call sequence
would time out and fail.

So actually we want to be in state (d), where both the provider and the
consumer implement phy_exit(). But we can only do that safely through
intermediary state (b), where the provider implements it first. This
effectively behaves just like (a), except it offers a safe migration
path for the consumer to call phy_exit() as mandated by the Generic PHY
API.

Extra development notes: ignoring the lynx_28g_power_on() error in
lynx_28g_exit() is a deliberate decision. The consumer can't deal with a
teardown path that is not error-free. Ignoring the error is not silent:
lynx_28g_power_on() -> lynx_28g_lane_reset() will print on failure.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Previously submitted as part of larger set:
https://lore.kernel.org/linux-phy/20260114152111.625350-9-vladimir.oltean@nxp.com/

Changes:
- Stop propagating lynx_28g_power_on() error code to lynx_28g_exit().
- Add more detailed explanation in commit message
---
 drivers/phy/freescale/phy-fsl-lynx-28g.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/phy/freescale/phy-fsl-lynx-28g.c b/drivers/phy/freescale/phy-fsl-lynx-28g.c
index b056951506dc..dd54d82a1e1f 100644
--- a/drivers/phy/freescale/phy-fsl-lynx-28g.c
+++ b/drivers/phy/freescale/phy-fsl-lynx-28g.c
@@ -1113,8 +1113,25 @@ static int lynx_28g_init(struct phy *phy)
 	return 0;
 }
 
+static int lynx_28g_exit(struct phy *phy)
+{
+	struct lynx_28g_lane *lane = phy_get_drvdata(phy);
+
+	/* The lane returns to the state where it isn't managed by the
+	 * consumer, so we must treat is as if it isn't initialized, and
+	 * always powered on.
+	 */
+	lane->init = false;
+	lane->powered_up = false;
+
+	lynx_28g_power_on(phy);
+
+	return 0;
+}
+
 static const struct phy_ops lynx_28g_ops = {
 	.init		= lynx_28g_init,
+	.exit		= lynx_28g_exit,
 	.power_on	= lynx_28g_power_on,
 	.power_off	= lynx_28g_power_off,
 	.set_mode	= lynx_28g_set_mode,
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH phy-next 1/3] phy: lynx-28g: use timeouts when waiting for lane halt and reset
  2026-03-21  1:14 ` [PATCH phy-next 1/3] phy: lynx-28g: use timeouts when waiting for lane halt and reset Vladimir Oltean
@ 2026-03-21 15:48   ` Andrew Lunn
  0 siblings, 0 replies; 5+ messages in thread
From: Andrew Lunn @ 2026-03-21 15:48 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: linux-phy, netdev, Ioana Ciornei, Vinod Koul, Neil Armstrong,
	Josua Mayer, linux-kernel

On Sat, Mar 21, 2026 at 03:14:49AM +0200, Vladimir Oltean wrote:
> There are various circumstances in which a lane halt, or a lane reset,
> will fail to complete. If this happens, it will hang the kernel, which
> only implements a busy loop with no timeout.
> 
> The circumstances in which this will happen are all bugs in nature:
> - if we try to power off a powered off lane
> - if we try to power off a lane that uses a PLL locked onto the wrong
>   refclk frequency (wrong RCW, but SoC boots anyway)
> 
> Actually, unbounded loops in the kernel are a bad practice, so let's use
> read_poll_timeout() with a custom function that reads both LNaTRSTCTL
> (lane transmit control register) and LNaRRSTCTL (lane receive control
> register) and returns true when the request is done in both directions.
> 
> The HLT_REQ bit has to clear, whereas the RST_DONE bit has to get set.
> 
> Any time such an error happens, it is catastrophic and there is no point
> in trying to propagate it to our callers:
> - if lynx_28g_set_mode() -> lynx_28g_power_on() times out, we have
>   already reconfigured the lane, but returning an error would tell the
>   caller that we didn't
> - if lynx_28g_power_off() times out, again not much for the consumer to
>   do to help get out of this situation - the phy_power_off() call is
>   probably made from a context that the consumer can't cancel, or it is
>   making it to return to a known state from a previous failure.
> 
> So just print an error if timeouts happen and let the driver control
> flow continue. The entire point is just to not let the kernel freeze.
> 
> Suggested-by: Josua Mayer <josua@solid-run.com>
> Link: https://lore.kernel.org/lkml/d0c8bbf8-a0c5-469f-a148-de2235948c0f@solid-run.com/
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-03-21 15:48 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-21  1:14 [PATCH phy-next 0/3] Lynx 28G: better init(), exit(), power_on(), power_off() Vladimir Oltean
2026-03-21  1:14 ` [PATCH phy-next 1/3] phy: lynx-28g: use timeouts when waiting for lane halt and reset Vladimir Oltean
2026-03-21 15:48   ` Andrew Lunn
2026-03-21  1:14 ` [PATCH phy-next 2/3] phy: lynx-28g: truly power the lanes up or down Vladimir Oltean
2026-03-21  1:14 ` [PATCH phy-next 3/3] phy: lynx-28g: implement phy_exit() operation Vladimir Oltean

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox