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

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

Posted by kr494167@gmail.com 2 days, 19 hours ago

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

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

Posted by Andrew Lunn 2 days, 9 hours ago

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

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

Posted by Vladimir Oltean 2 days, 15 hours ago

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.