[U-Boot] [PATCH] net: eth: Do sanity test on eth dev before eth_get_ops(dev)->start

[Joe Hershberger]

Hi Bin,

On Tue, Sep 8, 2015 at 10:44 AM, Bin Meng <bmeng.cn at gmail.com> wrote:
> Hi Joe,
>
> On Tue, Sep 8, 2015 at 11:32 PM, Joe Hershberger
> <joe.hershberger at gmail.com> wrote:
>> Hi Bin,
>>
>> On Sat, Sep 5, 2015 at 9:38 PM, Bin Meng <bmeng.cn at gmail.com> wrote:
>>> In eth_init(), eth_get_dev() can return NULL. We should do sanity
>>> test on eth dev before calling its start function.
>>>
>>> Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
>>> ---
>>>
>>>  net/eth.c | 4 ++++
>>>  1 file changed, 4 insertions(+)
>>>
>>> diff --git a/net/eth.c b/net/eth.c
>>> index 26520d3..6ec3a86 100644
>>> --- a/net/eth.c
>>> +++ b/net/eth.c
>>> @@ -370,6 +370,10 @@ int eth_init(void)
>>>                 eth_try_another(0);
>>>                 /* This will ensure the new "current" attempted to probe */
>>>                 current = eth_get_dev();
>>> +               if (!current) {
>>> +                       printf("No ethernet found.\n");
>>> +                       break;
>>> +               }
>>
>> I'm not sure I get the point of this. We already have a check above...
>>
>>         current = eth_get_dev();
>>         if (!current) {
>>                 printf("No ethernet found.\n");
>>                 return -ENODEV;
>>         }
>>
>
> But this does not help. Each time eth_get_dev() is called, current can
> be NULL as driver's probe can fail.

If that's the issue you are hitting it seems like you should attempt
to skip the device instead of printing the message. It doesn't make
sense to me to move to the next device and then print that there is no
Ethernet.

Also, this is fundamental to the eth subsystem. You should add a unit
test that fails in your case.

>>>         } while (old_current != current);
>>>
>>>         return ret;
>>> --
>
> Regards,
> Bin

Thanks,
-Joe