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

[Samuel Mendoza-Jonas <sam@mendozajonas.com>]

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