[PATCH net] net: fec: Fix possible NPD in fec_enet_phy_reset_after_clk_enable()

[PATCH net] net: fec: Fix possible NPD in fec_enet_phy_reset_after_clk_enable()

[Stefan Wahren]

The function of_phy_find_device may return NULL, so we need to take
care before dereferencing phy_dev.

Fixes: 64a632da538a ("net: fec: Fix phy_device lookup for phy_reset_after_clk_enable()")
Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
Cc: Christoph Niedermaier <cniedermaier@dh-electronics.com>
Cc: Richard Leitner <richard.leitner@skidata.com>
---
 drivers/net/ethernet/freescale/fec_main.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 1383918f8a3f..adf1f2bbcbb1 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -2363,7 +2363,8 @@ static void fec_enet_phy_reset_after_clk_enable(struct net_device *ndev)
 		 */
 		phy_dev = of_phy_find_device(fep->phy_node);
 		phy_reset_after_clk_enable(phy_dev);
-		put_device(&phy_dev->mdio.dev);
+		if (phy_dev)
+			put_device(&phy_dev->mdio.dev);
 	}
 }
 
-- 
2.34.1

Re: [PATCH net] net: fec: Fix possible NPD in fec_enet_phy_reset_after_clk_enable()

[Jakub Kicinski]

On Thu,  4 Sep 2025 11:13:34 +0200 Stefan Wahren wrote:
>  		phy_dev = of_phy_find_device(fep->phy_node);
>  		phy_reset_after_clk_enable(phy_dev);
> -		put_device(&phy_dev->mdio.dev);
> +		if (phy_dev)
> +			put_device(&phy_dev->mdio.dev);

Looks correct, but isn't it better to also wrap
phy_reset_after_clk_enable() with the if()?
Up to you..

Re: [PATCH net] net: fec: Fix possible NPD in fec_enet_phy_reset_after_clk_enable()

[Stefan Wahren]

Hi Jakub,

[drop bouncing address]

Am 04.09.25 um 15:58 schrieb Jakub Kicinski:
> On Thu,  4 Sep 2025 11:13:34 +0200 Stefan Wahren wrote:
>>   		phy_dev = of_phy_find_device(fep->phy_node);
>>   		phy_reset_after_clk_enable(phy_dev);
>> -		put_device(&phy_dev->mdio.dev);
>> +		if (phy_dev)
>> +			put_device(&phy_dev->mdio.dev);
> Looks correct, but isn't it better to also wrap
> phy_reset_after_clk_enable() with the if()?
since phy_reset_after_clk_enable() have an internal check, i thought it 
won't be necessary. So this variant has fewer lines.

Best regards
> Up to you..

Re: [PATCH net] net: fec: Fix possible NPD in fec_enet_phy_reset_after_clk_enable()

[Simon Horman]

On Thu, Sep 04, 2025 at 08:01:42PM +0200, Stefan Wahren wrote:
> Hi Jakub,
> 
> [drop bouncing address]
> 
> Am 04.09.25 um 15:58 schrieb Jakub Kicinski:
> > On Thu,  4 Sep 2025 11:13:34 +0200 Stefan Wahren wrote:
> > >   		phy_dev = of_phy_find_device(fep->phy_node);
> > >   		phy_reset_after_clk_enable(phy_dev);
> > > -		put_device(&phy_dev->mdio.dev);
> > > +		if (phy_dev) 
> > > +			put_device(&phy_dev->mdio.dev);
> > Looks correct, but isn't it better to also wrap
> > phy_reset_after_clk_enable() with the if()?
> since phy_reset_after_clk_enable() have an internal check, i thought it
> won't be necessary. So this variant has fewer lines.

FWIIW, I had the same thought as Jakub.
But I think it's a judgement call because, as Stefan says,
phy_reset_after_clk_enable() checks for NULL anyway.

Maybe one is easier to read than the other.
But I guess that is in the eye of the beholder.
And as we have this version I think it's just as well to stick with it.

Reviewed-by: Simon Horman <horms@kernel.org>

RE: [PATCH net] net: fec: Fix possible NPD in fec_enet_phy_reset_after_clk_enable()

[Wei Fang]

> The function of_phy_find_device may return NULL, so we need to take care
> before dereferencing phy_dev.
> 
> Fixes: 64a632da538a ("net: fec: Fix phy_device lookup for
> phy_reset_after_clk_enable()")
> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
> Cc: Christoph Niedermaier <cniedermaier@dh-electronics.com>
> Cc: Richard Leitner <richard.leitner@skidata.com>
> ---
>  drivers/net/ethernet/freescale/fec_main.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/freescale/fec_main.c
> b/drivers/net/ethernet/freescale/fec_main.c
> index 1383918f8a3f..adf1f2bbcbb1 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -2363,7 +2363,8 @@ static void
> fec_enet_phy_reset_after_clk_enable(struct net_device *ndev)
>  		 */
>  		phy_dev = of_phy_find_device(fep->phy_node);
>  		phy_reset_after_clk_enable(phy_dev);
> -		put_device(&phy_dev->mdio.dev);
> +		if (phy_dev)
> +			put_device(&phy_dev->mdio.dev);
>  	}
>  }
> 
> --
> 2.34.1

Thanks for fixing it.

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

Re: [PATCH net] net: fec: Fix possible NPD in fec_enet_phy_reset_after_clk_enable()

[patchwork-bot+netdevbpf]

Hello:

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

On Thu,  4 Sep 2025 11:13:34 +0200 you wrote:
> The function of_phy_find_device may return NULL, so we need to take
> care before dereferencing phy_dev.
> 
> Fixes: 64a632da538a ("net: fec: Fix phy_device lookup for phy_reset_after_clk_enable()")
> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
> Cc: Christoph Niedermaier <cniedermaier@dh-electronics.com>
> Cc: Richard Leitner <richard.leitner@skidata.com>
> 
> [...]

Here is the summary with links:
  - [net] net: fec: Fix possible NPD in fec_enet_phy_reset_after_clk_enable()
    https://git.kernel.org/netdev/net/c/03e79de4608b

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