Le 22/09/2025 à 14:53, Andrew Lunn a écrit :

On Mon, Sep 22, 2025 at 10:08:28AM +0200, Christophe JAILLET wrote:

quoted

If an error occurs after calling phy_connect_direct(), phy_disconnect()
should be called, as already done in the remove function.

Fixes: ed94493fb38a ("mv643xx_eth: convert to phylib")
Signed-off-by: Christophe JAILLET <redacted>
---
This patch is speculative and compile tested only.
Review with care.
---
  drivers/net/ethernet/marvell/mv643xx_eth.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c b/drivers/net/ethernet/marvell/mv643xx_eth.c
index 0ab52c57c648..de6a683d7afc 100644
--- a/drivers/net/ethernet/marvell/mv643xx_eth.c
+++ b/drivers/net/ethernet/marvell/mv643xx_eth.c

@@ -3263,6 +3263,8 @@ static int mv643xx_eth_probe(struct platform_device *pdev)
  	return 0;
  
  out:
+	if (dev->phydev)
+		phy_disconnect(dev->phydev);

This is correct, but it is a little bit less obviously correct than it
could be. Nothing in mv643xx_eth_probe sets dev->phydev. It happens
deep down in the call chain of of_phy_connect(). Just using:

	if (phydev)
		phy_disconnect(phydev);

would be more obvious for this probe function.

But since it is correct, Reviewed-by: Andrew Lunn [off-list ref]
and i will leave you to decide if you want to change it.

     Andrew

In the (pd->phy_addr != MV643XX_ETH_PHY_NONE) case, phydev could be an 
error code.

So, we should do something like:
	if (!IS_ERR_OR_NULL(phydev))
		phy_disconnect(phydev);

Agree?

CJ