[U-Boot] [PATCH] net: eth-uclass: ignore unavailable devices

[Michael Walle]

Am 2019-12-01 01:55, schrieb Alexandru Marginean:
> Hi Michael,
> 
> On 10/22/2019 1:03 AM, Michael Walle wrote:
>> device_probe() may fail in which case the seq_id will be -1. Don't
>> display these devices during startup. While this is only a cosmetic
>> change, the return value of eth_initialize() will also change to the
>> actual number of available devices. The return value is only used in
>> spl_net to decide whether there are any devices to boot from. So
>> returning only available devices is also more correct in that case.
>> 
>> Signed-off-by: Michael Walle <michael at walle.cc>
>> ---
>>   net/eth-uclass.c | 17 +++++++++++------
>>   1 file changed, 11 insertions(+), 6 deletions(-)
>> 
>> diff --git a/net/eth-uclass.c b/net/eth-uclass.c
>> index 3bd98b01ad..acd59216e3 100644
>> --- a/net/eth-uclass.c
>> +++ b/net/eth-uclass.c
>> @@ -420,20 +420,25 @@ int eth_initialize(void)
>>     		bootstage_mark(BOOTSTAGE_ID_NET_ETH_INIT);
>>   		do {
>> -			if (num_devices)
>> -				printf(", ");
>> +			if (dev->seq != -1) {
>> +				if (num_devices)
>> +					printf(", ");
>>   -			printf("eth%d: %s", dev->seq, dev->name);
>> +				printf("eth%d: %s", dev->seq, dev->name);
>>   -			if (ethprime && dev == prime_dev)
>> -				printf(" [PRIME]");
>> +				if (ethprime && dev == prime_dev)
>> +					printf(" [PRIME]");
>> +			}
>>     			eth_write_hwaddr(dev);
>>   +			if (dev->seq != -1)
>> +				num_devices++;
>>   			uclass_next_device_check(&dev);
>> -			num_devices++;
>>   		} while (dev);
>>   +		if (!num_devices)
>> +			printf("No ethernet found.\n");
>>   		putc('\n');
>>   	}
>> 
> 
> What would you say about something like this instead?  It's a bit more
> compact and should be functionally equivalent:

sure, but I've noticed that this is only part of the fix (or isn't even
necessary). At least, it doesn't work if the first device is 
unavailable,
because eth_get_dev() will then fail. You should be able to reproduce
the issue if you disable the first enetc of the LS1028A. Then network
should only work if ethact is set.

I've traced that back to the pci enumeration which doesn't respect the
'status = "disabled"' property. But instead someone added that check to
the enetc driver, but that doesn't really work, because the device is
already included in the uclass list. See separate patch, I'll send in a
minute.

-michael

> 
>  net/eth-uclass.c | 54 ++++++++++++++++++++++--------------------------
>  1 file changed, 25 insertions(+), 29 deletions(-)
> 
> diff --git a/net/eth-uclass.c b/net/eth-uclass.c
> index 1bc8947749..4d4eaeb371 100644
> --- a/net/eth-uclass.c
> +++ b/net/eth-uclass.c
> @@ -390,10 +390,16 @@ int eth_rx(void)
> 
>  int eth_initialize(void)
>  {
> +	struct udevice *dev, *prime_dev = NULL;
> +	char *ethprime = env_get("ethprime");
>  	int num_devices = 0;
> -	struct udevice *dev;
> +	struct uclass *uc;
> 
>  	eth_common_init();
> +	eth_set_dev(NULL);
> +
> +	if (ethprime)
> +		prime_dev = eth_get_dev_by_name(ethprime);
> 
>  	/*
>  	 * Devices need to write the hwaddr even if not started so that Linux
> @@ -401,40 +407,30 @@ int eth_initialize(void)
>  	 * This is accomplished by attempting to probe each device and 
> calling
>  	 * their write_hwaddr() operation.
>  	 */
> -	uclass_first_device_check(UCLASS_ETH, &dev);
> -	if (!dev) {
> -		printf("No ethernet found.\n");
> -		bootstage_error(BOOTSTAGE_ID_NET_ETH_START);
> -	} else {
> -		char *ethprime = env_get("ethprime");
> -		struct udevice *prime_dev = NULL;
> +	uclass_get(UCLASS_ETH, &uc);
> +	uclass_foreach_dev(dev, uc) {
> +		if (device_probe(dev))
> +			continue;
> +
> +		eth_write_hwaddr(dev);
> +
> +		if (num_devices)
> +			printf(", ");
> +		printf("eth%d: %s", dev->seq, dev->name);
> 
> -		if (ethprime)
> -			prime_dev = eth_get_dev_by_name(ethprime);
> -		if (prime_dev) {
> +		if (dev == prime_dev) {
>  			eth_set_dev(prime_dev);
>  			eth_current_changed();
> -		} else {
> -			eth_set_dev(NULL);
> +			printf(" [PRIME]");
>  		}
> +		num_devices++;
> +	}
> 
> +	if (!num_devices) {
> +		bootstage_error(BOOTSTAGE_ID_NET_ETH_START);
> +		printf("No ethernet found.\n");
> +	} else {
>  		bootstage_mark(BOOTSTAGE_ID_NET_ETH_INIT);
> -		do {
> -			if (device_active(dev)) {
> -				if (num_devices)
> -					printf(", ");
> -
> -				printf("eth%d: %s", dev->seq, dev->name);
> -
> -				if (ethprime && dev == prime_dev)
> -					printf(" [PRIME]");
> -				eth_write_hwaddr(dev);
> -			}
> -
> -			uclass_next_device_check(&dev);
> -			num_devices++;
> -		} while (dev);
> -
>  		putc('\n');
>  	}
> 
> Alex