diff mbox series

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

Message ID 20200109182802.26121-1-festevam@gmail.com
State Deferred
Delegated to: Joe Hershberger
Headers show
Series net: eth-uclass: Do not return error when no MAC is found | expand

Commit Message

Fabio Estevam Jan. 9, 2020, 6:28 p.m. UTC
On some i.MX8QXP MEK boards with no MAC address stored, the following
hang is seen:

Error: ethernet@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@5b040000 address not set.

Error: ethernet@5b040000 address 00:00:00:00:00:00 illegal value
eth0: ethernet@5b040000
Error: ethernet@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(-)

Comments

Fabio Estevam Feb. 13, 2020, 6:42 p.m. UTC | #1
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@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@5b040000 address not set.
>
> Error: ethernet@5b040000 address 00:00:00:00:00:00 illegal value
> eth0: ethernet@5b040000
> Error: ethernet@5b040000 address 00:00:00:00:00:00 illegal value
>
> Signed-off-by: Fabio Estevam <festevam@gmail.com>

Any comments?
Joe Hershberger March 5, 2020, 11:21 p.m. UTC | #2
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@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@5b040000 address not set.
>
> Error: ethernet@5b040000 address 00:00:00:00:00:00 illegal value
> eth0: ethernet@5b040000
> Error: ethernet@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>
Joe Hershberger March 10, 2020, 5:44 p.m. UTC | #3
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@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@5b040000 address not set.
>
> Error: ethernet@5b040000 address 00:00:00:00:00:00 illegal value
> eth0: ethernet@5b040000
> Error: ethernet@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
Fabio Estevam March 10, 2020, 6:34 p.m. UTC | #4
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
Joe Hershberger March 11, 2020, 5:14 p.m. UTC | #5
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
Fabio Estevam March 11, 2020, 5:19 p.m. UTC | #6
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
diff mbox series

Patch

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
 	}