[PATCH] net: eth-uclass: Do not return error when no MAC is found

[PATCH] net: eth-uclass: Do not return error when no MAC is found

[Fabio Estevam]

On some i.MX8QXP MEK boards with no MAC address stored, the following
hang is seen:

Error: ethernet at 5b040000 address not set.

(Board hangs)

One way to avoid this issue is to select CONFIG_NET_RANDOM_ETHADDR, so
that a random MAC is provided and boot proceeds, but the lack of MAC
should not hang the entire system.

Fix the hang by removing the propagation of error when no MAC is found.

In this case there are already messages indicating that something
is not correct:

Error: ethernet at 5b040000 address not set.

Error: ethernet at 5b040000 address 00:00:00:00:00:00 illegal value
eth0: ethernet at 5b040000
Error: ethernet at 5b040000 address 00:00:00:00:00:00 illegal value

Signed-off-by: Fabio Estevam <festevam@gmail.com>
---
 net/eth-uclass.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/net/eth-uclass.c b/net/eth-uclass.c
index ed81cbd537..bc2b0aac00 100644
--- a/net/eth-uclass.c
+++ b/net/eth-uclass.c
@@ -554,7 +554,6 @@ static int eth_post_probe(struct udevice *dev)
 #else
 		printf("\nError: %s address not set.\n",
 		       dev->name);
-		return -EINVAL;
 #endif
 	}
 
-- 
2.17.1

[PATCH] net: eth-uclass: Do not return error when no MAC is found

[Fabio Estevam]

Hi Joe,

On Thu, Jan 9, 2020 at 3:28 PM Fabio Estevam <festevam@gmail.com> wrote:
>
> On some i.MX8QXP MEK boards with no MAC address stored, the following
> hang is seen:
>
> Error: ethernet at 5b040000 address not set.
>
> (Board hangs)
>
> One way to avoid this issue is to select CONFIG_NET_RANDOM_ETHADDR, so
> that a random MAC is provided and boot proceeds, but the lack of MAC
> should not hang the entire system.
>
> Fix the hang by removing the propagation of error when no MAC is found.
>
> In this case there are already messages indicating that something
> is not correct:
>
> Error: ethernet at 5b040000 address not set.
>
> Error: ethernet at 5b040000 address 00:00:00:00:00:00 illegal value
> eth0: ethernet at 5b040000
> Error: ethernet at 5b040000 address 00:00:00:00:00:00 illegal value
>
> Signed-off-by: Fabio Estevam <festevam@gmail.com>

Any comments?

[PATCH] net: eth-uclass: Do not return error when no MAC is found

[Joe Hershberger]

On Thu, Jan 9, 2020 at 12:29 PM Fabio Estevam <festevam@gmail.com> wrote:
>
> On some i.MX8QXP MEK boards with no MAC address stored, the following
> hang is seen:
>
> Error: ethernet at 5b040000 address not set.
>
> (Board hangs)
>
> One way to avoid this issue is to select CONFIG_NET_RANDOM_ETHADDR, so
> that a random MAC is provided and boot proceeds, but the lack of MAC
> should not hang the entire system.
>
> Fix the hang by removing the propagation of error when no MAC is found.
>
> In this case there are already messages indicating that something
> is not correct:
>
> Error: ethernet at 5b040000 address not set.
>
> Error: ethernet at 5b040000 address 00:00:00:00:00:00 illegal value
> eth0: ethernet at 5b040000
> Error: ethernet at 5b040000 address 00:00:00:00:00:00 illegal value
>
> Signed-off-by: Fabio Estevam <festevam@gmail.com>

Acked-by: Joe Hershberger <joe.hershberger@ni.com>

[PATCH] net: eth-uclass: Do not return error when no MAC is found

[Joe Hershberger]

Hi Fabio,

On Thu, Jan 9, 2020 at 12:29 PM Fabio Estevam <festevam@gmail.com> wrote:
>
> On some i.MX8QXP MEK boards with no MAC address stored, the following
> hang is seen:
>
> Error: ethernet at 5b040000 address not set.
>
> (Board hangs)
>
> One way to avoid this issue is to select CONFIG_NET_RANDOM_ETHADDR, so
> that a random MAC is provided and boot proceeds, but the lack of MAC
> should not hang the entire system.
>
> Fix the hang by removing the propagation of error when no MAC is found.
>
> In this case there are already messages indicating that something
> is not correct:
>
> Error: ethernet at 5b040000 address not set.
>
> Error: ethernet at 5b040000 address 00:00:00:00:00:00 illegal value
> eth0: ethernet at 5b040000
> Error: ethernet at 5b040000 address 00:00:00:00:00:00 illegal value
>
> Signed-off-by: Fabio Estevam <festevam@gmail.com>

I tried to apply this but it breaks the unit tests. I guess you didn't
try "ut dm" in sandbox?

https://travis-ci.org/jhershbe/u-boot/builds/660395782

Please test those and update the tests to pass with your change.

Thanks,
-Joe

[PATCH] net: eth-uclass: Do not return error when no MAC is found

[Fabio Estevam]

Hi Joe,

On Tue, Mar 10, 2020 at 2:45 PM Joe Hershberger <joe.hershberger@ni.com> wrote:

> I tried to apply this but it breaks the unit tests. I guess you didn't
> try "ut dm" in sandbox?
>
> https://travis-ci.org/jhershbe/u-boot/builds/660395782
>
> Please test those and update the tests to pass with your change.

Ok, I will revisit this patch after 2020.04.

I think we should probably let net_random_ethaddr() run unconditionally:

--- a/net/eth-uclass.c
+++ b/net/eth-uclass.c
@@ -547,15 +547,9 @@ static int eth_post_probe(struct udevice *dev)
                       dev->name);
        } else if (is_zero_ethaddr(pdata->enetaddr) ||
                   !is_valid_ethaddr(pdata->enetaddr)) {
-#ifdef CONFIG_NET_RANDOM_ETHADDR
                net_random_ethaddr(pdata->enetaddr);
                printf("\nWarning: %s (eth%d) using random MAC address - %pM\n",
                       dev->name, dev->seq, pdata->enetaddr);
-#else
-               printf("\nError: %s address not set.\n",
-                      dev->name);
-               return -EINVAL;
-#endif
        }

        eth_write_hwaddr(dev);

Thanks

[PATCH] net: eth-uclass: Do not return error when no MAC is found

[Joe Hershberger]

On Tue, Mar 10, 2020 at 1:34 PM Fabio Estevam <festevam@gmail.com> wrote:
>
> Hi Joe,
>
> On Tue, Mar 10, 2020 at 2:45 PM Joe Hershberger <joe.hershberger@ni.com> wrote:
>
> > I tried to apply this but it breaks the unit tests. I guess you didn't
> > try "ut dm" in sandbox?
> >
> > https://travis-ci.org/jhershbe/u-boot/builds/660395782
> >
> > Please test those and update the tests to pass with your change.
>
> Ok, I will revisit this patch after 2020.04.
>
> I think we should probably let net_random_ethaddr() run unconditionally:

That may be a better solution... with the warning of course.

Cheers,
-Joe

> --- a/net/eth-uclass.c
> +++ b/net/eth-uclass.c
> @@ -547,15 +547,9 @@ static int eth_post_probe(struct udevice *dev)
>                        dev->name);
>         } else if (is_zero_ethaddr(pdata->enetaddr) ||
>                    !is_valid_ethaddr(pdata->enetaddr)) {
> -#ifdef CONFIG_NET_RANDOM_ETHADDR
>                 net_random_ethaddr(pdata->enetaddr);
>                 printf("\nWarning: %s (eth%d) using random MAC address - %pM\n",
>                        dev->name, dev->seq, pdata->enetaddr);
> -#else
> -               printf("\nError: %s address not set.\n",
> -                      dev->name);
> -               return -EINVAL;
> -#endif
>         }
>
>         eth_write_hwaddr(dev);
>
> Thanks

[PATCH] net: eth-uclass: Do not return error when no MAC is found

[Fabio Estevam]

Hi Joe,

On Wed, Mar 11, 2020 at 2:15 PM Joe Hershberger <joe.hershberger@ni.com> wrote:

> That may be a better solution... with the warning of course.

Thanks for the feedback.

I will submit this solution after 2020.04 is out.

Thanks