[PATCH] phy: fsl-lynx-28g: propagate PCVT enable errors

[PATCH] phy: fsl-lynx-28g: propagate PCVT enable errors

[kr494167]

From: surendra <kr494167@gmail.com>

lynx_28g_set_mode() currently ignores failures from
lynx_28g_lane_enable_pcvt(). It then updates the lane mode and reports
success even though the protocol converter may remain disabled.

Propagate the error and leave the previous lane mode intact so the caller
can handle the failed reconfiguration.

Fixes: 8f73b37cf3fb ("phy: add support for the Layerscape SerDes 28G")
Signed-off-by: surendra <kr494167@gmail.com>
---
 drivers/phy/freescale/phy-fsl-lynx-28g.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/phy/freescale/phy-fsl-lynx-28g.c b/drivers/phy/freescale/phy-fsl-lynx-28g.c
index 38afcd081a2a..1fe406adc83c 100644
--- a/drivers/phy/freescale/phy-fsl-lynx-28g.c
+++ b/drivers/phy/freescale/phy-fsl-lynx-28g.c
@@ -1003,7 +1003,9 @@ static int lynx_28g_set_mode(struct phy *phy, enum phy_mode mode, int submode)
 
 	lynx_28g_lane_change_proto_conf(lane, lane_mode);
 	lynx_28g_lane_remap_pll(lane, lane_mode);
-	WARN_ON(lynx_28g_lane_enable_pcvt(lane, lane_mode));
+	err = lynx_28g_lane_enable_pcvt(lane, lane_mode);
+	if (err)
+		goto out;
 
 	lane->mode = lane_mode;
 
-- 
2.55.0


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH] phy: fsl-lynx-28g: propagate PCVT enable errors

[Vladimir Oltean]

Hi Surendra,

On Wed, Jul 22, 2026 at 09:06:19AM +0530, kr494167@gmail.com wrote:
> From: surendra <kr494167@gmail.com>
> 
> lynx_28g_set_mode() currently ignores failures from
> lynx_28g_lane_enable_pcvt(). It then updates the lane mode and reports
> success even though the protocol converter may remain disabled.
> 
> Propagate the error and leave the previous lane mode intact so the caller
> can handle the failed reconfiguration.
> 
> Fixes: 8f73b37cf3fb ("phy: add support for the Layerscape SerDes 28G")
> Signed-off-by: surendra <kr494167@gmail.com>
> ---
>  drivers/phy/freescale/phy-fsl-lynx-28g.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/phy/freescale/phy-fsl-lynx-28g.c b/drivers/phy/freescale/phy-fsl-lynx-28g.c
> index 38afcd081a2a..1fe406adc83c 100644
> --- a/drivers/phy/freescale/phy-fsl-lynx-28g.c
> +++ b/drivers/phy/freescale/phy-fsl-lynx-28g.c
> @@ -1003,7 +1003,9 @@ static int lynx_28g_set_mode(struct phy *phy, enum phy_mode mode, int submode)
> 
>         lynx_28g_lane_change_proto_conf(lane, lane_mode);
>         lynx_28g_lane_remap_pll(lane, lane_mode);
> -       WARN_ON(lynx_28g_lane_enable_pcvt(lane, lane_mode));
> +       err = lynx_28g_lane_enable_pcvt(lane, lane_mode);
> +       if (err)
> +               goto out;
> 
>         lane->mode = lane_mode;
> 
> --
> 2.55.0
>

Did you draw any conclusions from the feedback you received on the
lynx-10g patch and decided that it doesn't apply to lynx-28g?
https://lore.kernel.org/linux-phy/20260720101906.80584-1-kr494167@gmail.com/

Hint: it does.

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH] phy: fsl-lynx-28g: propagate PCVT enable errors

[Andrew Lunn]

On Wed, Jul 22, 2026 at 09:06:19AM +0530, kr494167@gmail.com wrote:
> From: surendra <kr494167@gmail.com>
> 
> lynx_28g_set_mode() currently ignores failures from
> lynx_28g_lane_enable_pcvt(). It then updates the lane mode and reports
> success even though the protocol converter may remain disabled.
> 
> Propagate the error and leave the previous lane mode intact so the caller
> can handle the failed reconfiguration.
> 
> Fixes: 8f73b37cf3fb ("phy: add support for the Layerscape SerDes 28G")
> Signed-off-by: surendra <kr494167@gmail.com>
> ---
>  drivers/phy/freescale/phy-fsl-lynx-28g.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/phy/freescale/phy-fsl-lynx-28g.c b/drivers/phy/freescale/phy-fsl-lynx-28g.c
> index 38afcd081a2a..1fe406adc83c 100644
> --- a/drivers/phy/freescale/phy-fsl-lynx-28g.c
> +++ b/drivers/phy/freescale/phy-fsl-lynx-28g.c
> @@ -1003,7 +1003,9 @@ static int lynx_28g_set_mode(struct phy *phy, enum phy_mode mode, int submode)
>  
>  	lynx_28g_lane_change_proto_conf(lane, lane_mode);
>  	lynx_28g_lane_remap_pll(lane, lane_mode);
> -	WARN_ON(lynx_28g_lane_enable_pcvt(lane, lane_mode));

A WARN_ON() produces a very noticeable splat, which will aid
debugging. Just returning the error code makes is easy to miss what
causes the issue. Maybe the WARN_ON() should be kept? At least, i
would add justification to the commit message for removing it.

    Andrew

---
pw-bot: cr

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH] phy: fsl-lynx-28g: propagate PCVT enable errors

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Hardware and software state desynchronization on PCVT enable failure prevents mode recovery.
--

--- Patch [1]: [PATCH] phy: fsl-lynx-28g: propagate PCVT enable errors ---
commit 6193a7adcb18a183b9462da919d480f3a6949562
Author: surendra <kr494167@gmail.com>

phy: fsl-lynx-28g: propagate PCVT enable errors

This commit updates lynx_28g_set_mode() to check the return value of
lynx_28g_lane_enable_pcvt(). If an error occurs, the code now propagates
the error and exits before updating the software lane mode tracking.

> diff --git a/drivers/phy/freescale/phy-fsl-lynx-28g.c b/drivers/phy/freescale/phy-fsl-lynx-28g.c
> index 38afcd081a2a7..1fe406adc83cb 100644
> --- a/drivers/phy/freescale/phy-fsl-lynx-28g.c
> +++ b/drivers/phy/freescale/phy-fsl-lynx-28g.c
> @@ -1003,7 +1003,9 @@ static int lynx_28g_set_mode(struct phy *phy, enum phy_mode mode, int submode)
>  
>  	lynx_28g_lane_change_proto_conf(lane, lane_mode);
>  	lynx_28g_lane_remap_pll(lane, lane_mode);
> -	WARN_ON(lynx_28g_lane_enable_pcvt(lane, lane_mode));
> +	err = lynx_28g_lane_enable_pcvt(lane, lane_mode);
> +	if (err)
> +		goto out;

[Severity: Medium]
Does this new error path leave the software and hardware states
desynchronized?

Prior to this point, the hardware has already been reprogrammed to the
new mode by lynx_28g_lane_change_proto_conf() and
lynx_28g_lane_remap_pll(). If lynx_28g_lane_enable_pcvt() fails and takes
the goto out path, lane->mode remains set to the old mode.

If a caller later attempts to recover by setting the PHY back to the old
mode, lynx_28g_set_mode() will see:

    if (lane_mode == lane->mode)
        return 0;

and short-circuit, preventing the hardware from actually being reverted.

Should the error path either revert the hardware changes or set
lane->mode = LANE_MODE_UNKNOWN to ensure subsequent recovery attempts bypass
the short-circuit check?

>  
>  	lane->mode = lane_mode;
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722033619.10647-1-kr494167@gmail.com?part=1

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy