[PATCH v3] net: fec: fix pinctrl default state restore order on resume

[PATCH v3] net: fec: fix pinctrl default state restore order on resume

[Tapio Reijonen]

In fec_resume(), fec_enet_clk_enable() is called before
pinctrl_pm_select_default_state() in the non-WoL path, inverting the
ordering used in fec_suspend() which correctly switches to the sleep
pinctrl state before disabling clocks.

For PHYs with the PHY_RST_AFTER_CLK_EN flag (e.g. TI DP83848 or
SMSC LAN87xx), fec_enet_clk_enable() triggers a hardware reset pulse
via the phy-reset GPIO. With the GPIO pin still in sleep pinctrl state
at that point, the GPIO write has no physical effect and the PHY never
receives the required reset after clock enable, leading to unreliable
link establishment after system resume.

Fix by restoring the default pinctrl state before enabling clocks,
making resume the proper mirror of suspend. The call is made
unconditionally: fec_suspend() only switches to the sleep pinctrl state
on the non-WoL path and leaves the pins in the default state when WoL
is enabled, so on a WoL resume the device is already in the default
state and pinctrl_pm_select_default_state() is a no-op.

Fixes: de40ed31b3c5 ("net: fec: add Wake-on-LAN support")
Signed-off-by: Tapio Reijonen <tapio.reijonen@vaisala.com>
---
Changes in v3:
- Move pinctrl_pm_select_default_state() before fec_enet_clk_enable()
  unconditionally instead of duplicating the clk-enable call in both
  WoL branches; safe because fec_suspend() leaves the pins in the
  default state on the WoL path (per Wei Fang's review).
- Link to v2: https://lore.kernel.org/r/20260527-b4-fec-resume-pinctrl-order-v2-1-293096f48703@vaisala.com

Changes in v2:
- Remove a stray blank line between the Fixes: tag and Signed-off-by:
  in the commit message; no functional change (per Wei Fang's review).
- Link to v1: https://lore.kernel.org/r/20260526-b4-fec-resume-pinctrl-order-v1-1-f2f926325424@vaisala.com
---
 drivers/net/ethernet/freescale/fec_main.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index f89aa94ce0202d5f28f37362ce70e0943aa14025..6ebde65d7f1b8728a6c5d28f75cc0b0c4d983969 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -5594,6 +5594,7 @@ static int fec_resume(struct device *dev)
 		if (fep->rpm_active)
 			pm_runtime_force_resume(dev);
 
+		pinctrl_pm_select_default_state(&fep->pdev->dev);
 		ret = fec_enet_clk_enable(ndev, true);
 		if (ret) {
 			rtnl_unlock();
@@ -5610,8 +5611,6 @@ static int fec_resume(struct device *dev)
 			val &= ~(FEC_ECR_MAGICEN | FEC_ECR_SLEEP);
 			writel(val, fep->hwp + FEC_ECNTRL);
 			fep->wol_flag &= ~FEC_WOL_FLAG_SLEEP_ON;
-		} else {
-			pinctrl_pm_select_default_state(&fep->pdev->dev);
 		}
 		fec_restart(ndev);
 		netif_tx_lock_bh(ndev);

---
base-commit: 79bd2dded182b1d458b18e62684b7f82ffc682e5
change-id: 20260526-b4-fec-resume-pinctrl-order-fde0cff2bbff

Best regards,
-- 
Tapio Reijonen <tapio.reijonen@vaisala.com>

RE: [PATCH v3] net: fec: fix pinctrl default state restore order on resume

[Wei Fang]

>  drivers/net/ethernet/freescale/fec_main.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/fec_main.c
> b/drivers/net/ethernet/freescale/fec_main.c
> index
> f89aa94ce0202d5f28f37362ce70e0943aa14025..6ebde65d7f1b8728a6c5d28f7
> 5cc0b0c4d983969 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -5594,6 +5594,7 @@ static int fec_resume(struct device *dev)
>  		if (fep->rpm_active)
>  			pm_runtime_force_resume(dev);
> 
> +		pinctrl_pm_select_default_state(&fep->pdev->dev);
>  		ret = fec_enet_clk_enable(ndev, true);
>  		if (ret) {
>  			rtnl_unlock();
> @@ -5610,8 +5611,6 @@ static int fec_resume(struct device *dev)
>  			val &= ~(FEC_ECR_MAGICEN | FEC_ECR_SLEEP);
>  			writel(val, fep->hwp + FEC_ECNTRL);
>  			fep->wol_flag &= ~FEC_WOL_FLAG_SLEEP_ON;
> -		} else {
> -			pinctrl_pm_select_default_state(&fep->pdev->dev);
>  		}
>  		fec_restart(ndev);
>  		netif_tx_lock_bh(ndev);
> 

It looks good to me, thanks for fixing this issue.

Reviewed-by: Wei Fang <wei.fang@nxp.com>

By the way, it seems you posted two identical patches, which might
cause confusion.

Re: [PATCH v3] net: fec: fix pinctrl default state restore order on resume

[Tapio Reijonen]

Hi Wei,

On 5/29/26 11:37, Wei Fang wrote:
>>   drivers/net/ethernet/freescale/fec_main.c | 3 +--
>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/freescale/fec_main.c
>> b/drivers/net/ethernet/freescale/fec_main.c
>> index
>> f89aa94ce0202d5f28f37362ce70e0943aa14025..6ebde65d7f1b8728a6c5d28f7
>> 5cc0b0c4d983969 100644
>> --- a/drivers/net/ethernet/freescale/fec_main.c
>> +++ b/drivers/net/ethernet/freescale/fec_main.c
>> @@ -5594,6 +5594,7 @@ static int fec_resume(struct device *dev)
>>   		if (fep->rpm_active)
>>   			pm_runtime_force_resume(dev);
>>
>> +		pinctrl_pm_select_default_state(&fep->pdev->dev);
>>   		ret = fec_enet_clk_enable(ndev, true);
>>   		if (ret) {
>>   			rtnl_unlock();
>> @@ -5610,8 +5611,6 @@ static int fec_resume(struct device *dev)
>>   			val &= ~(FEC_ECR_MAGICEN | FEC_ECR_SLEEP);
>>   			writel(val, fep->hwp + FEC_ECNTRL);
>>   			fep->wol_flag &= ~FEC_WOL_FLAG_SLEEP_ON;
>> -		} else {
>> -			pinctrl_pm_select_default_state(&fep->pdev->dev);
>>   		}
>>   		fec_restart(ndev);
>>   		netif_tx_lock_bh(ndev);
>>
> 
> It looks good to me, thanks for fixing this issue.
> 
> Reviewed-by: Wei Fang <wei.fang@nxp.com>
> 
> By the way, it seems you posted two identical patches, which might
> cause confusion.
> 
Yes, that was my mistake -- sorry for the noise.
I aborted the first 'b4 sned' with Ctrl-C, but it looks like the message 
had already been handed off to the SMTP server by that point, so it went 
out anyway.
I'll be more careful next time.

Thanks for the review!

Best regards,
Tapio Reijonen

Re: [PATCH v3] net: fec: fix pinctrl default state restore order on resume

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Fri, 29 May 2026 06:18:57 +0000 you wrote:
> In fec_resume(), fec_enet_clk_enable() is called before
> pinctrl_pm_select_default_state() in the non-WoL path, inverting the
> ordering used in fec_suspend() which correctly switches to the sleep
> pinctrl state before disabling clocks.
> 
> For PHYs with the PHY_RST_AFTER_CLK_EN flag (e.g. TI DP83848 or
> SMSC LAN87xx), fec_enet_clk_enable() triggers a hardware reset pulse
> via the phy-reset GPIO. With the GPIO pin still in sleep pinctrl state
> at that point, the GPIO write has no physical effect and the PHY never
> receives the required reset after clock enable, leading to unreliable
> link establishment after system resume.
> 
> [...]

Here is the summary with links:
  - [v3] net: fec: fix pinctrl default state restore order on resume
    https://git.kernel.org/netdev/net/c/b455410146bf

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

[PATCH v3] net: fec: fix pinctrl default state restore order on resume

[Tapio Reijonen]

In fec_resume(), fec_enet_clk_enable() is called before
pinctrl_pm_select_default_state() in the non-WoL path, inverting the
ordering used in fec_suspend() which correctly switches to the sleep
pinctrl state before disabling clocks.

For PHYs with the PHY_RST_AFTER_CLK_EN flag (e.g. TI DP83848 or
SMSC LAN87xx), fec_enet_clk_enable() triggers a hardware reset pulse
via the phy-reset GPIO. With the GPIO pin still in sleep pinctrl state
at that point, the GPIO write has no physical effect and the PHY never
receives the required reset after clock enable, leading to unreliable
link establishment after system resume.

Fix by restoring the default pinctrl state before enabling clocks,
making resume the proper mirror of suspend. The call is made
unconditionally: fec_suspend() only switches to the sleep pinctrl state
on the non-WoL path and leaves the pins in the default state when WoL
is enabled, so on a WoL resume the device is already in the default
state and pinctrl_pm_select_default_state() is a no-op.

Fixes: de40ed31b3c5 ("net: fec: add Wake-on-LAN support")
Signed-off-by: Tapio Reijonen <tapio.reijonen@vaisala.com>
---
Changes in v3:
- Move pinctrl_pm_select_default_state() before fec_enet_clk_enable()
  unconditionally instead of duplicating the clk-enable call in both
  WoL branches; safe because fec_suspend() leaves the pins in the
  default state on the WoL path (per Wei Fang's review).
- Link to v2: https://lore.kernel.org/r/20260527-b4-fec-resume-pinctrl-order-v2-1-293096f48703@vaisala.com

Changes in v2:
- Remove a stray blank line between the Fixes: tag and Signed-off-by:
  in the commit message; no functional change (per Wei Fang's review).
- Link to v1: https://lore.kernel.org/r/20260526-b4-fec-resume-pinctrl-order-v1-1-f2f926325424@vaisala.com
---
 drivers/net/ethernet/freescale/fec_main.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index f89aa94ce0202d5f28f37362ce70e0943aa14025..6ebde65d7f1b8728a6c5d28f75cc0b0c4d983969 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -5594,6 +5594,7 @@ static int fec_resume(struct device *dev)
 		if (fep->rpm_active)
 			pm_runtime_force_resume(dev);
 
+		pinctrl_pm_select_default_state(&fep->pdev->dev);
 		ret = fec_enet_clk_enable(ndev, true);
 		if (ret) {
 			rtnl_unlock();
@@ -5610,8 +5611,6 @@ static int fec_resume(struct device *dev)
 			val &= ~(FEC_ECR_MAGICEN | FEC_ECR_SLEEP);
 			writel(val, fep->hwp + FEC_ECNTRL);
 			fep->wol_flag &= ~FEC_WOL_FLAG_SLEEP_ON;
-		} else {
-			pinctrl_pm_select_default_state(&fep->pdev->dev);
 		}
 		fec_restart(ndev);
 		netif_tx_lock_bh(ndev);

---
base-commit: 79bd2dded182b1d458b18e62684b7f82ffc682e5
change-id: 20260526-b4-fec-resume-pinctrl-order-fde0cff2bbff

Best regards,
-- 
Tapio Reijonen <tapio.reijonen@vaisala.com>