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

[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:

  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