Hi Joe,
On Wed, Sep 9, 2015 at 12:01 AM, Joe Hershberger
<joe.hershber...@gmail.com> wrote:
> Hi Bin,
>
> On Tue, Sep 8, 2015 at 10:44 AM, Bin Meng <bmeng...@gmail.com> wrote:
>> Hi Joe,
>>
>> On Tue, Sep 8, 2015 at 11:32 PM, Joe Hershberger
>> <joe.hershber...@gmail.com> wrote:
>>> Hi Bin,
>>>
>>> On Sat, Sep 5, 2015 at 9:38 PM, Bin Meng <bmeng...@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...@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.
Do you mean we should not printf("No ethernet found.\n") and just break here?
If it fails, U-Boot just crashes as there is a NULL pointer. I am not
sure if test case is able to handle this?
>
> 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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
