[PATCH] mdio_bus: Fix MDIO bus scanning in __mdiobus_register()

[PATCH] mdio_bus: Fix MDIO bus scanning in __mdiobus_register()

[Marek Vasut]

Since commit b74766a0a0feeef5c779709cc5d109451c0d5b17 in linux-next,
( phylib: don't return NULL from get_phy_device() ), phy_get_device()
will return ERR_PTR(-ENODEV) instead of NULL if the PHY device ID is
all ones.

This causes problem with stmmac driver and likely some other drivers
which call mdiobus_register(). I triggered this bug on SoCFPGA MCVEVK
board with linux-next 20160427 and 20160428. In case of the stmmac, if
there is no PHY node specified in the DT for the stmmac block, the stmmac
driver ( drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c function
stmmac_mdio_register() ) will call mdiobus_register() , which will
register the MDIO bus and probe for the PHY.

The mdiobus_register() resp. __mdiobus_register() iterates over all of
the addresses on the MDIO bus and calls mdiobus_scan() for each of them,
which invokes get_phy_device(). Before the aforementioned patch, the
mdiobus_scan() would return NULL if no PHY was found on a given address
and mdiobus_register() would continue and try the next PHY address. Now,
mdiobus_scan() returns ERR_PTR(-ENODEV), which is caught by the
'if (IS_ERR(phydev))' condition and the loop exits immediatelly if the
PHY address does not contain PHY.

Repair this by explicitly checking for the ERR_PTR(-ENODEV) and if this
error comes around, continue with the next PHY address.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: David S. Miller <davem@davemloft.net>
Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
---
 drivers/net/phy/mdio_bus.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

NOTE: I don't quite like this explicit check , but I don't have better idea now.

diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index 499003ee..388f992 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -333,7 +333,7 @@ int __mdiobus_register(struct mii_bus *bus, struct module *owner)
 			struct phy_device *phydev;
 
 			phydev = mdiobus_scan(bus, i);
-			if (IS_ERR(phydev)) {
+			if (IS_ERR(phydev) && (PTR_ERR(phydev) != -ENODEV)) {
 				err = PTR_ERR(phydev);
 				goto error;
 			}
-- 
2.7.0

Re: [PATCH] mdio_bus: Fix MDIO bus scanning in __mdiobus_register()

[Florian Fainelli]

Le 28/04/2016 18:09, Marek Vasut a écrit :
> Since commit b74766a0a0feeef5c779709cc5d109451c0d5b17 in linux-next,
> ( phylib: don't return NULL from get_phy_device() ), phy_get_device()
> will return ERR_PTR(-ENODEV) instead of NULL if the PHY device ID is
> all ones.
> 
> This causes problem with stmmac driver and likely some other drivers
> which call mdiobus_register(). I triggered this bug on SoCFPGA MCVEVK
> board with linux-next 20160427 and 20160428. In case of the stmmac, if
> there is no PHY node specified in the DT for the stmmac block, the stmmac
> driver ( drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c function
> stmmac_mdio_register() ) will call mdiobus_register() , which will
> register the MDIO bus and probe for the PHY.
> 
> The mdiobus_register() resp. __mdiobus_register() iterates over all of
> the addresses on the MDIO bus and calls mdiobus_scan() for each of them,
> which invokes get_phy_device(). Before the aforementioned patch, the
> mdiobus_scan() would return NULL if no PHY was found on a given address
> and mdiobus_register() would continue and try the next PHY address. Now,
> mdiobus_scan() returns ERR_PTR(-ENODEV), which is caught by the
> 'if (IS_ERR(phydev))' condition and the loop exits immediatelly if the
> PHY address does not contain PHY.
> 
> Repair this by explicitly checking for the ERR_PTR(-ENODEV) and if this
> error comes around, continue with the next PHY address.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
> Cc: Florian Fainelli <f.fainelli@gmail.com>
> Cc: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

Acked-by: Florian Fainelli <f.fainelli@gmail.com>

I had an exact same patch posted yesterday but not formally like you
did, thanks!

> ---
>  drivers/net/phy/mdio_bus.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> NOTE: I don't quite like this explicit check , but I don't have better idea now.
> 
> diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
> index 499003ee..388f992 100644
> --- a/drivers/net/phy/mdio_bus.c
> +++ b/drivers/net/phy/mdio_bus.c
> @@ -333,7 +333,7 @@ int __mdiobus_register(struct mii_bus *bus, struct module *owner)
>  			struct phy_device *phydev;
>  
>  			phydev = mdiobus_scan(bus, i);
> -			if (IS_ERR(phydev)) {
> +			if (IS_ERR(phydev) && (PTR_ERR(phydev) != -ENODEV)) {
>  				err = PTR_ERR(phydev);
>  				goto error;
>  			}
> 


-- 
Florian

Re: [PATCH] mdio_bus: Fix MDIO bus scanning in __mdiobus_register()

[Marek Vasut]

On 04/29/2016 03:49 AM, Florian Fainelli wrote:
> Le 28/04/2016 18:09, Marek Vasut a écrit :
>> Since commit b74766a0a0feeef5c779709cc5d109451c0d5b17 in linux-next,
>> ( phylib: don't return NULL from get_phy_device() ), phy_get_device()
>> will return ERR_PTR(-ENODEV) instead of NULL if the PHY device ID is
>> all ones.
>>
>> This causes problem with stmmac driver and likely some other drivers
>> which call mdiobus_register(). I triggered this bug on SoCFPGA MCVEVK
>> board with linux-next 20160427 and 20160428. In case of the stmmac, if
>> there is no PHY node specified in the DT for the stmmac block, the stmmac
>> driver ( drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c function
>> stmmac_mdio_register() ) will call mdiobus_register() , which will
>> register the MDIO bus and probe for the PHY.
>>
>> The mdiobus_register() resp. __mdiobus_register() iterates over all of
>> the addresses on the MDIO bus and calls mdiobus_scan() for each of them,
>> which invokes get_phy_device(). Before the aforementioned patch, the
>> mdiobus_scan() would return NULL if no PHY was found on a given address
>> and mdiobus_register() would continue and try the next PHY address. Now,
>> mdiobus_scan() returns ERR_PTR(-ENODEV), which is caught by the
>> 'if (IS_ERR(phydev))' condition and the loop exits immediatelly if the
>> PHY address does not contain PHY.
>>
>> Repair this by explicitly checking for the ERR_PTR(-ENODEV) and if this
>> error comes around, continue with the next PHY address.
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> Cc: Arnd Bergmann <arnd@arndb.de>
>> Cc: David S. Miller <davem@davemloft.net>
>> Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
>> Cc: Florian Fainelli <f.fainelli@gmail.com>
>> Cc: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> 
> Acked-by: Florian Fainelli <f.fainelli@gmail.com>
> 
> I had an exact same patch posted yesterday but not formally like you
> did, thanks!

Ah, my google-fu must be weak tonight. Thanks!

>> ---
>>  drivers/net/phy/mdio_bus.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> NOTE: I don't quite like this explicit check , but I don't have better idea now.
>>
>> diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
>> index 499003ee..388f992 100644
>> --- a/drivers/net/phy/mdio_bus.c
>> +++ b/drivers/net/phy/mdio_bus.c
>> @@ -333,7 +333,7 @@ int __mdiobus_register(struct mii_bus *bus, struct module *owner)
>>  			struct phy_device *phydev;
>>  
>>  			phydev = mdiobus_scan(bus, i);
>> -			if (IS_ERR(phydev)) {
>> +			if (IS_ERR(phydev) && (PTR_ERR(phydev) != -ENODEV)) {
>>  				err = PTR_ERR(phydev);
>>  				goto error;
>>  			}
>>
> 
> 


-- 
Best regards,
Marek Vasut

Re: [PATCH] mdio_bus: Fix MDIO bus scanning in __mdiobus_register()

[Sergei Shtylyov]

Hello.

    First of all, thank you for the patch!
    You beat me to it (and not only me). :-)

On 4/29/2016 4:09 AM, Marek Vasut wrote:

> Since commit b74766a0a0feeef5c779709cc5d109451c0d5b17 in linux-next,
> ( phylib: don't return NULL from get_phy_device() ), phy_get_device()

    scripts/checkpatch.pl now enforces certain commit citing style, yours 
doesn't quite match it.

> will return ERR_PTR(-ENODEV) instead of NULL if the PHY device ID is
> all ones.
>
> This causes problem with stmmac driver and likely some other drivers
> which call mdiobus_register(). I triggered this bug on SoCFPGA MCVEVK
> board with linux-next 20160427 and 20160428. In case of the stmmac, if
> there is no PHY node specified in the DT for the stmmac block, the stmmac
> driver ( drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c function
> stmmac_mdio_register() ) will call mdiobus_register() , which will
> register the MDIO bus and probe for the PHY.
>
> The mdiobus_register() resp. __mdiobus_register() iterates over all of
> the addresses on the MDIO bus and calls mdiobus_scan() for each of them,
> which invokes get_phy_device(). Before the aforementioned patch, the
> mdiobus_scan() would return NULL if no PHY was found on a given address
> and mdiobus_register() would continue and try the next PHY address. Now,
> mdiobus_scan() returns ERR_PTR(-ENODEV), which is caught by the
> 'if (IS_ERR(phydev))' condition and the loop exits immediatelly if the
> PHY address does not contain PHY.
>
> Repair this by explicitly checking for the ERR_PTR(-ENODEV) and if this
> error comes around, continue with the next PHY address.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
> Cc: Florian Fainelli <f.fainelli@gmail.com>
> Cc: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

Reviewed-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

> ---
>  drivers/net/phy/mdio_bus.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> NOTE: I don't quite like this explicit check , but I don't have better idea now.

    It's fine. I was going to do just the same :-)

> diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
> index 499003ee..388f992 100644
> --- a/drivers/net/phy/mdio_bus.c
> +++ b/drivers/net/phy/mdio_bus.c
> @@ -333,7 +333,7 @@ int __mdiobus_register(struct mii_bus *bus, struct module *owner)
>  			struct phy_device *phydev;
>
>  			phydev = mdiobus_scan(bus, i);
> -			if (IS_ERR(phydev)) {
> +			if (IS_ERR(phydev) && (PTR_ERR(phydev) != -ENODEV)) {

    Parens around the second operand of && are not really needed though...

[...]

MBR, Sergei

Re: [PATCH] mdio_bus: Fix MDIO bus scanning in __mdiobus_register()

[Marek Vasut]

On 04/29/2016 01:18 PM, Sergei Shtylyov wrote:
> Hello.

Hi!

>    First of all, thank you for the patch!
>    You beat me to it (and not only me). :-)

Heh, hacking at night has it's perks :)

> On 4/29/2016 4:09 AM, Marek Vasut wrote:
> 
>> Since commit b74766a0a0feeef5c779709cc5d109451c0d5b17 in linux-next,
>> ( phylib: don't return NULL from get_phy_device() ), phy_get_device()
> 
>    scripts/checkpatch.pl now enforces certain commit citing style, yours
> doesn't quite match it.

Ha, I didn't know that checkpatch can now warn about this too, nice. Is
that in next already ? I just tried checkpatch and it doesn't warn about it.

Anyway, regarding this format, do you want V2 ? Originally, I had the
full commit info in the message, but that was just taking space and
it is not the commit which is important in the message, so I trimmed
it down.

>> will return ERR_PTR(-ENODEV) instead of NULL if the PHY device ID is
>> all ones.
>>
>> This causes problem with stmmac driver and likely some other drivers
>> which call mdiobus_register(). I triggered this bug on SoCFPGA MCVEVK
>> board with linux-next 20160427 and 20160428. In case of the stmmac, if
>> there is no PHY node specified in the DT for the stmmac block, the stmmac
>> driver ( drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c function
>> stmmac_mdio_register() ) will call mdiobus_register() , which will
>> register the MDIO bus and probe for the PHY.
>>
>> The mdiobus_register() resp. __mdiobus_register() iterates over all of
>> the addresses on the MDIO bus and calls mdiobus_scan() for each of them,
>> which invokes get_phy_device(). Before the aforementioned patch, the
>> mdiobus_scan() would return NULL if no PHY was found on a given address
>> and mdiobus_register() would continue and try the next PHY address. Now,
>> mdiobus_scan() returns ERR_PTR(-ENODEV), which is caught by the
>> 'if (IS_ERR(phydev))' condition and the loop exits immediatelly if the
>> PHY address does not contain PHY.
>>
>> Repair this by explicitly checking for the ERR_PTR(-ENODEV) and if this
>> error comes around, continue with the next PHY address.
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> Cc: Arnd Bergmann <arnd@arndb.de>
>> Cc: David S. Miller <davem@davemloft.net>
>> Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
>> Cc: Florian Fainelli <f.fainelli@gmail.com>
>> Cc: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> 
> Reviewed-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> 
>> ---
>>  drivers/net/phy/mdio_bus.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> NOTE: I don't quite like this explicit check , but I don't have better
>> idea now.
> 
>    It's fine. I was going to do just the same :-)

OK, I'm glad I'm not alone on this one :)

>> diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
>> index 499003ee..388f992 100644
>> --- a/drivers/net/phy/mdio_bus.c
>> +++ b/drivers/net/phy/mdio_bus.c
>> @@ -333,7 +333,7 @@ int __mdiobus_register(struct mii_bus *bus, struct
>> module *owner)
>>              struct phy_device *phydev;
>>
>>              phydev = mdiobus_scan(bus, i);
>> -            if (IS_ERR(phydev)) {
>> +            if (IS_ERR(phydev) && (PTR_ERR(phydev) != -ENODEV)) {
> 
>    Parens around the second operand of && are not really needed though...

While I agree, I also prefer to make things obvious when reading the
code by adding the parenthesis. It's a matter of taste I think. Just let
me know if I should spin V2 without them :)

Thanks for the review!

> [...]
> 
> MBR, Sergei
> 


-- 
Best regards,
Marek Vasut

Re: [PATCH] mdio_bus: Fix MDIO bus scanning in __mdiobus_register()

[Sergei Shtylyov]

Hello.

On 04/29/2016 02:45 PM, Marek Vasut wrote:

>>     First of all, thank you for the patch!
>>     You beat me to it (and not only me). :-)
>
> Heh, hacking at night has it's perks :)

    I know, I'm just supposed to work on different things. :-)

>> On 4/29/2016 4:09 AM, Marek Vasut wrote:
>>
>>> Since commit b74766a0a0feeef5c779709cc5d109451c0d5b17 in linux-next,
>>> ( phylib: don't return NULL from get_phy_device() ), phy_get_device()
>>
>>     scripts/checkpatch.pl now enforces certain commit citing style, yours
>> doesn't quite match it.
>
> Ha, I didn't know that checkpatch can now warn about this too, nice. Is
> that in next already ? I just tried checkpatch and it doesn't warn about it.

    It's been merged long ago. Actually, I've just run this script from the 
current Linus' tree on your patch, here's the results:

ERROR: Please use git commit description style 'commit <12+ chars of sha1> 
("<title line>")' - ie: 'commit fatal: bad o ("cc5d109451c0d5b17")'
#4:
Since commit b74766a0a0feeef5c779709cc5d109451c0d5b17 in linux-next,

WARNING: 'immediatelly' may be misspelled - perhaps 'immediately'?
#23:
'if (IS_ERR(phydev))' condition and the loop exits immediatelly if the

total: 1 errors, 1 warnings, 0 checks, 8 lines checked

> Anyway, regarding this format, do you want V2 ? Originally, I had the
> full commit info in the message, but that was just taking space and
> it is not the commit which is important in the message, so I trimmed
> it down.

    I'll let DaveM decide.

>>> will return ERR_PTR(-ENODEV) instead of NULL if the PHY device ID is
>>> all ones.
>>>
>>> This causes problem with stmmac driver and likely some other drivers
>>> which call mdiobus_register(). I triggered this bug on SoCFPGA MCVEVK
>>> board with linux-next 20160427 and 20160428. In case of the stmmac, if
>>> there is no PHY node specified in the DT for the stmmac block, the stmmac
>>> driver ( drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c function
>>> stmmac_mdio_register() ) will call mdiobus_register() , which will
>>> register the MDIO bus and probe for the PHY.
>>>
>>> The mdiobus_register() resp. __mdiobus_register() iterates over all of
>>> the addresses on the MDIO bus and calls mdiobus_scan() for each of them,
>>> which invokes get_phy_device(). Before the aforementioned patch, the
>>> mdiobus_scan() would return NULL if no PHY was found on a given address
>>> and mdiobus_register() would continue and try the next PHY address. Now,
>>> mdiobus_scan() returns ERR_PTR(-ENODEV), which is caught by the
>>> 'if (IS_ERR(phydev))' condition and the loop exits immediatelly if the
>>> PHY address does not contain PHY.
>>>
>>> Repair this by explicitly checking for the ERR_PTR(-ENODEV) and if this
>>> error comes around, continue with the next PHY address.
>>>
>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>> Cc: Arnd Bergmann <arnd@arndb.de>
>>> Cc: David S. Miller <davem@davemloft.net>
>>> Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
>>> Cc: Florian Fainelli <f.fainelli@gmail.com>
>>> Cc: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>>
>> Reviewed-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>>
>>> ---
>>>   drivers/net/phy/mdio_bus.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> NOTE: I don't quite like this explicit check , but I don't have better
>>> idea now.
>>
>>     It's fine. I was going to do just the same :-)
>
> OK, I'm glad I'm not alone on this one :)
>
>>> diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
>>> index 499003ee..388f992 100644
>>> --- a/drivers/net/phy/mdio_bus.c
>>> +++ b/drivers/net/phy/mdio_bus.c
>>> @@ -333,7 +333,7 @@ int __mdiobus_register(struct mii_bus *bus, struct
>>> module *owner)
>>>               struct phy_device *phydev;
>>>
>>>               phydev = mdiobus_scan(bus, i);
>>> -            if (IS_ERR(phydev)) {
>>> +            if (IS_ERR(phydev) && (PTR_ERR(phydev) != -ENODEV)) {
>>
>>     Parens around the second operand of && are not really needed though...
>
> While I agree, I also prefer to make things obvious when reading the
> code by adding the parenthesis. It's a matter of taste I think. Just let
> me know if I should spin V2 without them :)

    Again, let DaveM decide. But I don't think the code being patched uses 
parens in such cases... I wouldn't, for sure.

> Thanks for the review!

    You've saved me some work (but I still need to analyze the other call paths).

>> [...]

MBR, Sergei

Re: [PATCH] mdio_bus: Fix MDIO bus scanning in __mdiobus_register()

[Marek Vasut]

On 04/29/2016 05:46 PM, Sergei Shtylyov wrote:
> Hello.

Hi!

> On 04/29/2016 02:45 PM, Marek Vasut wrote:
> 
>>>     First of all, thank you for the patch!
>>>     You beat me to it (and not only me). :-)
>>
>> Heh, hacking at night has it's perks :)
> 
>    I know, I'm just supposed to work on different things. :-)
> 
>>> On 4/29/2016 4:09 AM, Marek Vasut wrote:
>>>
>>>> Since commit b74766a0a0feeef5c779709cc5d109451c0d5b17 in linux-next,
>>>> ( phylib: don't return NULL from get_phy_device() ), phy_get_device()
>>>
>>>     scripts/checkpatch.pl now enforces certain commit citing style,
>>> yours
>>> doesn't quite match it.
>>
>> Ha, I didn't know that checkpatch can now warn about this too, nice. Is
>> that in next already ? I just tried checkpatch and it doesn't warn
>> about it.
> 
>    It's been merged long ago. Actually, I've just run this script from
> the current Linus' tree on your patch, here's the results:

Oh, got it now.

> ERROR: Please use git commit description style 'commit <12+ chars of
> sha1> ("<title line>")' - ie: 'commit fatal: bad o ("cc5d109451c0d5b17")'
> #4:
> Since commit b74766a0a0feeef5c779709cc5d109451c0d5b17 in linux-next,
> 
> WARNING: 'immediatelly' may be misspelled - perhaps 'immediately'?
> #23:
> 'if (IS_ERR(phydev))' condition and the loop exits immediatelly if the
> 
> total: 1 errors, 1 warnings, 0 checks, 8 lines checked

Ha, this is embarrassing .

>> Anyway, regarding this format, do you want V2 ? Originally, I had the
>> full commit info in the message, but that was just taking space and
>> it is not the commit which is important in the message, so I trimmed
>> it down.
> 
>    I'll let DaveM decide.

OK

>>>> will return ERR_PTR(-ENODEV) instead of NULL if the PHY device ID is
>>>> all ones.
>>>>
>>>> This causes problem with stmmac driver and likely some other drivers
>>>> which call mdiobus_register(). I triggered this bug on SoCFPGA MCVEVK
>>>> board with linux-next 20160427 and 20160428. In case of the stmmac, if
>>>> there is no PHY node specified in the DT for the stmmac block, the
>>>> stmmac
>>>> driver ( drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c function
>>>> stmmac_mdio_register() ) will call mdiobus_register() , which will
>>>> register the MDIO bus and probe for the PHY.
>>>>
>>>> The mdiobus_register() resp. __mdiobus_register() iterates over all of
>>>> the addresses on the MDIO bus and calls mdiobus_scan() for each of
>>>> them,
>>>> which invokes get_phy_device(). Before the aforementioned patch, the
>>>> mdiobus_scan() would return NULL if no PHY was found on a given address
>>>> and mdiobus_register() would continue and try the next PHY address.
>>>> Now,
>>>> mdiobus_scan() returns ERR_PTR(-ENODEV), which is caught by the
>>>> 'if (IS_ERR(phydev))' condition and the loop exits immediatelly if the
>>>> PHY address does not contain PHY.
>>>>
>>>> Repair this by explicitly checking for the ERR_PTR(-ENODEV) and if this
>>>> error comes around, continue with the next PHY address.
>>>>
>>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>>> Cc: Arnd Bergmann <arnd@arndb.de>
>>>> Cc: David S. Miller <davem@davemloft.net>
>>>> Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
>>>> Cc: Florian Fainelli <f.fainelli@gmail.com>
>>>> Cc: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>>>
>>> Reviewed-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>>>
>>>> ---
>>>>   drivers/net/phy/mdio_bus.c | 2 +-
>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> NOTE: I don't quite like this explicit check , but I don't have better
>>>> idea now.
>>>
>>>     It's fine. I was going to do just the same :-)
>>
>> OK, I'm glad I'm not alone on this one :)
>>
>>>> diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
>>>> index 499003ee..388f992 100644
>>>> --- a/drivers/net/phy/mdio_bus.c
>>>> +++ b/drivers/net/phy/mdio_bus.c
>>>> @@ -333,7 +333,7 @@ int __mdiobus_register(struct mii_bus *bus, struct
>>>> module *owner)
>>>>               struct phy_device *phydev;
>>>>
>>>>               phydev = mdiobus_scan(bus, i);
>>>> -            if (IS_ERR(phydev)) {
>>>> +            if (IS_ERR(phydev) && (PTR_ERR(phydev) != -ENODEV)) {
>>>
>>>     Parens around the second operand of && are not really needed
>>> though...
>>
>> While I agree, I also prefer to make things obvious when reading the
>> code by adding the parenthesis. It's a matter of taste I think. Just let
>> me know if I should spin V2 without them :)
> 
>    Again, let DaveM decide. But I don't think the code being patched
> uses parens in such cases... I wouldn't, for sure.
> 
>> Thanks for the review!
> 
>    You've saved me some work (but I still need to analyze the other call
> paths).

If there is something you want to test on the stmmac, let me know.

>>> [...]
> 
> MBR, Sergei
> 


-- 
Best regards,
Marek Vasut

Re: [PATCH] mdio_bus: Fix MDIO bus scanning in __mdiobus_register()

[David Miller]

Please respin this with the checkpatch errors fixed.

I don't have any opinion about the conditional parenthesis, so
don't worry about that.

Re: [PATCH] mdio_bus: Fix MDIO bus scanning in __mdiobus_register()

[Marek Vasut]

On 05/02/2016 01:49 AM, David Miller wrote:
> 
> Please respin this with the checkpatch errors fixed.

Done and sent.

> I don't have any opinion about the conditional parenthesis, so
> don't worry about that.
> 
OK

Thanks!

-- 
Best regards,
Marek Vasut