of: check for IS_ERR()

get_phy_device() can return an ERR_PTR()

---
I don't have a cross compile environment set up so I can't even compile test this. :/ But err.h is included so it should be OK.

index 18ecae4..b474833 100644
@@ -69,7 +69,7 @@ int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np)
 		}
 
 		phy = get_phy_device(mdio, be32_to_cpup(addr));
-		if (!phy) {
+		if (!phy || IS_ERR(phy)) {
 			dev_err(&mdio->dev, "error probing PHY at address %i\n",
 				*addr);
 			continue;

From: Dan Carpenter <[email protected]>
Date: Fri, 26 Feb 2010 12:49:41 +0300

It should return ERR_PTR() consistently. Checking for both NULL and ERR_PTR() is undesirable.

Ugh. This is why I dislike the ERR_PTR() pattern so much. The compiler cannot do any type checking and it is implemented inconsistently. You have to go look at the calling function to find out what you're allowed to do with the return value. ie. which test do I use? (!ptr) or IS_ERR(ptr)?

It would be better if ERR_PTR() returned a structure or a union. At least that way the compiler would yell at you if the an ERR_PTR was being returned.

g.

Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Hi Giulio,

get_phy_device() currently returns NULL because of: 3ee82383f0098a2 "phy:
fix phy address bug". If I change it to return ERR_PTR(-ENODEV) that will mean we break out of the loop with an error in mdiobus_register() where before we would just continue on.

drivers/net/phy/mdio_bus.c
119 phydev = mdiobus_scan(bus, i);
120 if (IS_ERR(phydev)) {
121 err = PTR_ERR(phydev);
122 goto error;
123 }

Is that OK?

I'm not really familiar with this hardware at all, I'm just going based on static analysis. :/

regards,
dan carpenter