[U-Boot] [PATCH 2/2] [v3] net: abort network initialization if the PHY driver fails

[Timur Tabi]

Joe Hershberger wrote:

>>>> @@ -384,7 +387,11 @@ static int fm_eth_open(struct eth_device *dev, bd_t *bd)
>>>>         fmc_tx_port_graceful_stop_disable(fm_eth);
>>>>
>>>>  #ifdef CONFIG_PHYLIB
>>>> -       phy_startup(fm_eth->phydev);
>>>> +       ret = phy_startup(fm_eth->phydev);
>>>> +       if (ret) {
>>>> +               printf("%s: Could not initialize\n", fm_eth->phydev->dev->name);
>>>
>>> Why is this string different from the others? Consistency?
>>
>> Yes.  I tried to keep the messages consistent with the other messages in
>> the function.
> 
> Should you not at least keep the core message the same?  "Could not
> initialize PHY"

Well, I suppose I could add the word "PHY" here.

>>>> diff --git a/drivers/net/sh_eth.c b/drivers/net/sh_eth.c
>>>> index bb57e4d..268d884 100644
>>>> --- a/drivers/net/sh_eth.c
>>>> +++ b/drivers/net/sh_eth.c
>>>> @@ -415,7 +415,11 @@ static int sh_eth_config(struct sh_eth_dev *eth, bd_t *bd)
>>>>                 goto err_phy_cfg;
>>>>         }
>>>>         phy = port_info->phydev;
>>>> -       phy_startup(phy);
>>>> +       ret = phy_startup(phy);
>>>> +       if (ret) {
>>>> +               printf(SHETHER_NAME ": phy startup failure\n");
>>>
>>> Why is this string different from the others?  Consistency?
>>
>> Yes, it looks like the other messages in sh_eth_config().
> 
> Same here, at least the core message "Could not initialize PHY"

I am saying that, just in a different way.  "phy startup failure" means
the same as "Could not initialize PHY", but the wording matches the rest
of the function.

-- 
Timur Tabi
Linux kernel developer at Freescale