diff mbox series

[U-Boot,v2,4/4] net: NC-SI setup and handling

Message ID 20190618013720.2823-5-sam@mendozajonas.com
State Deferred
Delegated to: Joe Hershberger
Headers show
Series NC-SI PHY Support | expand

Commit Message

Sam Mendoza-Jonas June 18, 2019, 1:37 a.m. UTC 
Add the handling of NC-SI ethernet frames, and add a check at the start
of net_loop() to configure NC-SI before starting other network commands.
This also adds an "ncsi" command to manually start NC-SI configuration.

Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>
---
v2: Update ncsi command help text

 cmd/Kconfig   |  8 ++++++++
 cmd/net.c     | 18 ++++++++++++++++++
 include/net.h |  2 +-
 net/net.c     | 27 ++++++++++++++++++++++++++-
 4 files changed, 53 insertions(+), 2 deletions(-)

Comments

Joel Stanley June 19, 2019, 2:35 a.m. UTC | #1 
On Tue, 18 Jun 2019 at 01:38, Samuel Mendoza-Jonas <sam@mendozajonas.com> wrote:
>
> Add the handling of NC-SI ethernet frames, and add a check at the start
> of net_loop() to configure NC-SI before starting other network commands.
> This also adds an "ncsi" command to manually start NC-SI configuration.
>
> Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>
> ---
> v2: Update ncsi command help text
>
>  cmd/Kconfig   |  8 ++++++++
>  cmd/net.c     | 18 ++++++++++++++++++
>  include/net.h |  2 +-
>  net/net.c     | 27 ++++++++++++++++++++++++++-
>  4 files changed, 53 insertions(+), 2 deletions(-)
>
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index 0badcb3fe0..9851b04972 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -1332,6 +1332,14 @@ config CMD_LINK_LOCAL
>         help
>           Acquire a network IP address using the link-local protocol
>
> +config CMD_NCSI
> +       bool "ncsi"
> +       depends on PHY_NCSI
> +       help
> +         Manually configure the attached NIC via NC-SI.
> +         Normally this happens automatically before other network
> +         operations.

Do we need to make it even clearer that "this command not required to use NCSI"?

I'm thinking of future me wondering if we should put this in openbmc
builds or not. It would be good to have your advice on this.

Cheers,

Joel
Sam Mendoza-Jonas June 20, 2019, 3:35 a.m. UTC | #2 
On Wed, 2019-06-19 at 02:35 +0000, Joel Stanley wrote:
> On Tue, 18 Jun 2019 at 01:38, Samuel Mendoza-Jonas <sam@mendozajonas.com> wrote:
> > Add the handling of NC-SI ethernet frames, and add a check at the start
> > of net_loop() to configure NC-SI before starting other network commands.
> > This also adds an "ncsi" command to manually start NC-SI configuration.
> > 
> > Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>
> > ---
> > v2: Update ncsi command help text
> > 
> >  cmd/Kconfig   |  8 ++++++++
> >  cmd/net.c     | 18 ++++++++++++++++++
> >  include/net.h |  2 +-
> >  net/net.c     | 27 ++++++++++++++++++++++++++-
> >  4 files changed, 53 insertions(+), 2 deletions(-)
> > 
> > diff --git a/cmd/Kconfig b/cmd/Kconfig
> > index 0badcb3fe0..9851b04972 100644
> > --- a/cmd/Kconfig
> > +++ b/cmd/Kconfig
> > @@ -1332,6 +1332,14 @@ config CMD_LINK_LOCAL
> >         help
> >           Acquire a network IP address using the link-local protocol
> > 
> > +config CMD_NCSI
> > +       bool "ncsi"
> > +       depends on PHY_NCSI
> > +       help
> > +         Manually configure the attached NIC via NC-SI.
> > +         Normally this happens automatically before other network
> > +         operations.
> 
> Do we need to make it even clearer that "this command not required to use NCSI"?
> 
> I'm thinking of future me wondering if we should put this in openbmc
> builds or not. It would be good to have your advice on this.
> 
> Cheers,
> 
> Joel

Either way; it's definitely not required but might be useful for sanity
checking things in BMC bringup.
Joe Hershberger July 9, 2019, 11:33 p.m. UTC | #3 
On Mon, Jun 17, 2019 at 8:43 PM Samuel Mendoza-Jonas
<sam@mendozajonas.com> wrote:
>
> Add the handling of NC-SI ethernet frames, and add a check at the start
> of net_loop() to configure NC-SI before starting other network commands.
> This also adds an "ncsi" command to manually start NC-SI configuration.
>
> Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>
> ---
> v2: Update ncsi command help text
>
>  cmd/Kconfig   |  8 ++++++++
>  cmd/net.c     | 18 ++++++++++++++++++
>  include/net.h |  2 +-
>  net/net.c     | 27 ++++++++++++++++++++++++++-
>  4 files changed, 53 insertions(+), 2 deletions(-)
>
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index 0badcb3fe0..9851b04972 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -1332,6 +1332,14 @@ config CMD_LINK_LOCAL
>         help
>           Acquire a network IP address using the link-local protocol
>
> +config CMD_NCSI
> +       bool "ncsi"
> +       depends on PHY_NCSI
> +       help
> +         Manually configure the attached NIC via NC-SI.
> +         Normally this happens automatically before other network
> +         operations.
> +
>  endif
>
>  config CMD_ETHSW
> diff --git a/cmd/net.c b/cmd/net.c
> index 89721b8f8b..7d2c21ba4d 100644
> --- a/cmd/net.c
> +++ b/cmd/net.c
> @@ -457,3 +457,21 @@ U_BOOT_CMD(
>  );
>
>  #endif  /* CONFIG_CMD_LINK_LOCAL */
> +
> +#if defined(CONFIG_CMD_NCSI)
> +static int do_ncsi(cmd_tbl_t *cmdtp, int flag, int argc,
> +                  char * const argv[])
> +{
> +       if (net_loop(NCSI) < 0)
> +               return CMD_RET_FAILURE;
> +
> +       return CMD_RET_SUCCESS;
> +}
> +
> +U_BOOT_CMD(
> +       ncsi,   1,      1,      do_ncsi,
> +       "Configure attached NIC via NC-SI",
> +       ""
> +);
> +
> +#endif  /* CONFIG_CMD_NCSI */
> diff --git a/include/net.h b/include/net.h
> index 20c448ee60..9189f0fb53 100644
> --- a/include/net.h
> +++ b/include/net.h
> @@ -531,7 +531,7 @@ extern int          net_restart_wrap;       /* Tried all network devices */
>
>  enum proto_t {
>         BOOTP, RARP, ARP, TFTPGET, DHCP, PING, DNS, NFS, CDP, NETCONS, SNTP,
> -       TFTPSRV, TFTPPUT, LINKLOCAL, FASTBOOT, WOL
> +       TFTPSRV, TFTPPUT, LINKLOCAL, FASTBOOT, WOL, NCSI
>  };
>
>  extern char    net_boot_file_name[1024];/* Boot File name */
> diff --git a/net/net.c b/net/net.c
> index 58b0417cbe..a59169a032 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -95,6 +95,7 @@
>  #include <net.h>
>  #include <net/fastboot.h>
>  #include <net/tftp.h>
> +#include <net/ncsi.h>
>  #if defined(CONFIG_LED_STATUS)
>  #include <miiphy.h>
>  #include <status_led.h>
> @@ -407,6 +408,16 @@ int net_loop(enum proto_t protocol)
>         net_try_count = 1;
>         debug_cond(DEBUG_INT_STATE, "--- net_loop Entry\n");
>
> +#ifdef CONFIG_PHY_NCSI
> +       if (protocol != NCSI && !ncsi_active()) {
> +               printf("%s: configuring NCSI first\n", __func__);

NC-SI

> +               if (net_loop(NCSI) < 0)
> +                       return ret;
> +               eth_init_state_only();
> +               goto restart;
> +       }
> +#endif
> +
>         bootstage_mark_name(BOOTSTAGE_ID_ETH_START, "eth_start");
>         net_init();
>         if (eth_is_on_demand_init() || protocol != NETCONS) {
> @@ -420,6 +431,7 @@ int net_loop(enum proto_t protocol)
>         } else {
>                 eth_init_state_only();
>         }
> +
>  restart:
>  #ifdef CONFIG_USB_KEYBOARD
>         net_busy_flag = 0;
> @@ -526,6 +538,11 @@ restart:
>                 case WOL:
>                         wol_start();
>                         break;
> +#endif
> +#if defined(CONFIG_CMD_NCSI)
> +               case NCSI:
> +                       ncsi_probe_packages();
> +                       break;
>  #endif
>                 default:
>                         break;
> @@ -637,7 +654,7 @@ restart:
>                                 env_set_hex("filesize", net_boot_file_size);
>                                 env_set_hex("fileaddr", load_addr);
>                         }
> -                       if (protocol != NETCONS)
> +                       if (protocol != NETCONS && protocol != NCSI)

Why is this using eth_halt_state_only()? It is not using the
eth_init_state_only().

>                                 eth_halt();
>                         else
>                                 eth_halt_state_only();
> @@ -1318,6 +1335,11 @@ void net_process_received_packet(uchar *in_packet, int len)
>         case PROT_WOL:
>                 wol_receive(ip, len);
>                 break;
> +#endif
> +#ifdef CONFIG_PHY_NCSI
> +       case PROT_NCSI:
> +               ncsi_receive(et, ip, len);
> +               break;
>  #endif
>         }
>  }
> @@ -1379,6 +1401,9 @@ common:
>
>  #ifdef CONFIG_CMD_RARP
>         case RARP:
> +#endif
> +#ifdef CONFIG_CMD_NCSI
> +       case NCSI:
>  #endif
>         case BOOTP:
>         case CDP:
> --
> 2.21.0
>
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://lists.denx.de/listinfo/u-boot
Sam Mendoza-Jonas Sept. 9, 2019, 2:34 a.m. UTC | #4 
On Tue, 2019-07-09 at 23:33 +0000, Joe Hershberger wrote:
> On Mon, Jun 17, 2019 at 8:43 PM Samuel Mendoza-Jonas
> <sam@mendozajonas.com> wrote:
> > Add the handling of NC-SI ethernet frames, and add a check at the
> > start
> > of net_loop() to configure NC-SI before starting other network
> > commands.
> > This also adds an "ncsi" command to manually start NC-SI
> > configuration.
> > 
> > Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>
> > ---
> > v2: Update ncsi command help text
> > 
> >  cmd/Kconfig   |  8 ++++++++
> >  cmd/net.c     | 18 ++++++++++++++++++
> >  include/net.h |  2 +-
> >  net/net.c     | 27 ++++++++++++++++++++++++++-
> >  4 files changed, 53 insertions(+), 2 deletions(-)
> > 
> > diff --git a/cmd/Kconfig b/cmd/Kconfig
> > index 0badcb3fe0..9851b04972 100644
> > --- a/cmd/Kconfig
> > +++ b/cmd/Kconfig
> > @@ -1332,6 +1332,14 @@ config CMD_LINK_LOCAL
> >         help
> >           Acquire a network IP address using the link-local
> > protocol
> > 
> > +config CMD_NCSI
> > +       bool "ncsi"
> > +       depends on PHY_NCSI
> > +       help
> > +         Manually configure the attached NIC via NC-SI.
> > +         Normally this happens automatically before other network
> > +         operations.
> > +
> >  endif
> > 
> >  config CMD_ETHSW
> > diff --git a/cmd/net.c b/cmd/net.c
> > index 89721b8f8b..7d2c21ba4d 100644
> > --- a/cmd/net.c
> > +++ b/cmd/net.c
> > @@ -457,3 +457,21 @@ U_BOOT_CMD(
> >  );
> > 
> >  #endif  /* CONFIG_CMD_LINK_LOCAL */
> > +
> > +#if defined(CONFIG_CMD_NCSI)
> > +static int do_ncsi(cmd_tbl_t *cmdtp, int flag, int argc,
> > +                  char * const argv[])
> > +{
> > +       if (net_loop(NCSI) < 0)
> > +               return CMD_RET_FAILURE;
> > +
> > +       return CMD_RET_SUCCESS;
> > +}
> > +
> > +U_BOOT_CMD(
> > +       ncsi,   1,      1,      do_ncsi,
> > +       "Configure attached NIC via NC-SI",
> > +       ""
> > +);
> > +
> > +#endif  /* CONFIG_CMD_NCSI */
> > diff --git a/include/net.h b/include/net.h
> > index 20c448ee60..9189f0fb53 100644
> > --- a/include/net.h
> > +++ b/include/net.h
> > @@ -531,7 +531,7 @@ extern int          net_restart_wrap;       /*
> > Tried all network devices */
> > 
> >  enum proto_t {
> >         BOOTP, RARP, ARP, TFTPGET, DHCP, PING, DNS, NFS, CDP,
> > NETCONS, SNTP,
> > -       TFTPSRV, TFTPPUT, LINKLOCAL, FASTBOOT, WOL
> > +       TFTPSRV, TFTPPUT, LINKLOCAL, FASTBOOT, WOL, NCSI
> >  };
> > 
> >  extern char    net_boot_file_name[1024];/* Boot File name */
> > diff --git a/net/net.c b/net/net.c
> > index 58b0417cbe..a59169a032 100644
> > --- a/net/net.c
> > +++ b/net/net.c
> > @@ -95,6 +95,7 @@
> >  #include <net.h>
> >  #include <net/fastboot.h>
> >  #include <net/tftp.h>
> > +#include <net/ncsi.h>
> >  #if defined(CONFIG_LED_STATUS)
> >  #include <miiphy.h>
> >  #include <status_led.h>
> > @@ -407,6 +408,16 @@ int net_loop(enum proto_t protocol)
> >         net_try_count = 1;
> >         debug_cond(DEBUG_INT_STATE, "--- net_loop Entry\n");
> > 
> > +#ifdef CONFIG_PHY_NCSI
> > +       if (protocol != NCSI && !ncsi_active()) {
> > +               printf("%s: configuring NCSI first\n", __func__);
> 
> NC-SI
> 
> > +               if (net_loop(NCSI) < 0)
> > +                       return ret;
> > +               eth_init_state_only();
> > +               goto restart;
> > +       }
> > +#endif
> > +
> >         bootstage_mark_name(BOOTSTAGE_ID_ETH_START, "eth_start");
> >         net_init();
> >         if (eth_is_on_demand_init() || protocol != NETCONS) {
> > @@ -420,6 +431,7 @@ int net_loop(enum proto_t protocol)
> >         } else {
> >                 eth_init_state_only();
> >         }
> > +
> >  restart:
> >  #ifdef CONFIG_USB_KEYBOARD
> >         net_busy_flag = 0;
> > @@ -526,6 +538,11 @@ restart:
> >                 case WOL:
> >                         wol_start();
> >                         break;
> > +#endif
> > +#if defined(CONFIG_CMD_NCSI)
> > +               case NCSI:
> > +                       ncsi_probe_packages();
> > +                       break;
> >  #endif
> >                 default:
> >                         break;
> > @@ -637,7 +654,7 @@ restart:
> >                                 env_set_hex("filesize",
> > net_boot_file_size);
> >                                 env_set_hex("fileaddr", load_addr);
> >                         }
> > -                       if (protocol != NETCONS)
> > +                       if (protocol != NETCONS && protocol !=
> > NCSI)
> 
> Why is this using eth_halt_state_only()? It is not using the
> eth_init_state_only().

The aim here was to avoid completely halting the network state because
we're about to start the net_loop again with whatever protocol caused
NC-SI to need to be configured. I'm new to this part of u-boot however
so it's possible I've misinterpeted the actions here; the NC-SI model
is an odd fit for the network loop.

> 
> >                                 eth_halt();
> >                         else
> >                                 eth_halt_state_only();
> > @@ -1318,6 +1335,11 @@ void net_process_received_packet(uchar
> > *in_packet, int len)
> >         case PROT_WOL:
> >                 wol_receive(ip, len);
> >                 break;
> > +#endif
> > +#ifdef CONFIG_PHY_NCSI
> > +       case PROT_NCSI:
> > +               ncsi_receive(et, ip, len);
> > +               break;
> >  #endif
> >         }
> >  }
> > @@ -1379,6 +1401,9 @@ common:
> > 
> >  #ifdef CONFIG_CMD_RARP
> >         case RARP:
> > +#endif
> > +#ifdef CONFIG_CMD_NCSI
> > +       case NCSI:
> >  #endif
> >         case BOOTP:
> >         case CDP:
> > --
> > 2.21.0
> > 
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot@lists.denx.de
> > https://lists.denx.de/listinfo/u-boot
diff mbox series

Patch

diff --git a/cmd/Kconfig b/cmd/Kconfig
index 0badcb3fe0..9851b04972 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -1332,6 +1332,14 @@  config CMD_LINK_LOCAL
 	help
 	  Acquire a network IP address using the link-local protocol
 
+config CMD_NCSI
+	bool "ncsi"
+	depends on PHY_NCSI
+	help
+	  Manually configure the attached NIC via NC-SI.
+	  Normally this happens automatically before other network
+	  operations.
+
 endif
 
 config CMD_ETHSW
diff --git a/cmd/net.c b/cmd/net.c
index 89721b8f8b..7d2c21ba4d 100644
--- a/cmd/net.c
+++ b/cmd/net.c
@@ -457,3 +457,21 @@  U_BOOT_CMD(
 );
 
 #endif  /* CONFIG_CMD_LINK_LOCAL */
+
+#if defined(CONFIG_CMD_NCSI)
+static int do_ncsi(cmd_tbl_t *cmdtp, int flag, int argc,
+		   char * const argv[])
+{
+	if (net_loop(NCSI) < 0)
+		return CMD_RET_FAILURE;
+
+	return CMD_RET_SUCCESS;
+}
+
+U_BOOT_CMD(
+	ncsi,	1,	1,	do_ncsi,
+	"Configure attached NIC via NC-SI",
+	""
+);
+
+#endif  /* CONFIG_CMD_NCSI */
diff --git a/include/net.h b/include/net.h
index 20c448ee60..9189f0fb53 100644
--- a/include/net.h
+++ b/include/net.h
@@ -531,7 +531,7 @@  extern int		net_restart_wrap;	/* Tried all network devices */
 
 enum proto_t {
 	BOOTP, RARP, ARP, TFTPGET, DHCP, PING, DNS, NFS, CDP, NETCONS, SNTP,
-	TFTPSRV, TFTPPUT, LINKLOCAL, FASTBOOT, WOL
+	TFTPSRV, TFTPPUT, LINKLOCAL, FASTBOOT, WOL, NCSI
 };
 
 extern char	net_boot_file_name[1024];/* Boot File name */
diff --git a/net/net.c b/net/net.c
index 58b0417cbe..a59169a032 100644
--- a/net/net.c
+++ b/net/net.c
@@ -95,6 +95,7 @@ 
 #include <net.h>
 #include <net/fastboot.h>
 #include <net/tftp.h>
+#include <net/ncsi.h>
 #if defined(CONFIG_LED_STATUS)
 #include <miiphy.h>
 #include <status_led.h>
@@ -407,6 +408,16 @@  int net_loop(enum proto_t protocol)
 	net_try_count = 1;
 	debug_cond(DEBUG_INT_STATE, "--- net_loop Entry\n");
 
+#ifdef CONFIG_PHY_NCSI
+	if (protocol != NCSI && !ncsi_active()) {
+		printf("%s: configuring NCSI first\n", __func__);
+		if (net_loop(NCSI) < 0)
+			return ret;
+		eth_init_state_only();
+		goto restart;
+	}
+#endif
+
 	bootstage_mark_name(BOOTSTAGE_ID_ETH_START, "eth_start");
 	net_init();
 	if (eth_is_on_demand_init() || protocol != NETCONS) {
@@ -420,6 +431,7 @@  int net_loop(enum proto_t protocol)
 	} else {
 		eth_init_state_only();
 	}
+
 restart:
 #ifdef CONFIG_USB_KEYBOARD
 	net_busy_flag = 0;
@@ -526,6 +538,11 @@  restart:
 		case WOL:
 			wol_start();
 			break;
+#endif
+#if defined(CONFIG_CMD_NCSI)
+		case NCSI:
+			ncsi_probe_packages();
+			break;
 #endif
 		default:
 			break;
@@ -637,7 +654,7 @@  restart:
 				env_set_hex("filesize", net_boot_file_size);
 				env_set_hex("fileaddr", load_addr);
 			}
-			if (protocol != NETCONS)
+			if (protocol != NETCONS && protocol != NCSI)
 				eth_halt();
 			else
 				eth_halt_state_only();
@@ -1318,6 +1335,11 @@  void net_process_received_packet(uchar *in_packet, int len)
 	case PROT_WOL:
 		wol_receive(ip, len);
 		break;
+#endif
+#ifdef CONFIG_PHY_NCSI
+	case PROT_NCSI:
+		ncsi_receive(et, ip, len);
+		break;
 #endif
 	}
 }
@@ -1379,6 +1401,9 @@  common:
 
 #ifdef CONFIG_CMD_RARP
 	case RARP:
+#endif
+#ifdef CONFIG_CMD_NCSI
+	case NCSI:
 #endif
 	case BOOTP:
 	case CDP: