[PATCH v3 0/3] Enable NC-SI support

[PATCH v3 0/3] Enable NC-SI support

[Joel Stanley]

Back in 2019 Sam submitted NC-SI support. The NC-SI PHY driver was
merged (patches 1 and 2), but we never got around to merging patches 3
and 4:

 https://lore.kernel.org/u-boot/20190618013720.2823-1-sam@mendozajonas.com/

Sam as long since moved on from working on the Aspeed BMCs, but the code
has been in use in the vendor fork for some time.

This refreshes his patches and enables support in the Aspeed defconfigs,
giving compile coverage to the NC-SI phy.

I've called the series v3 to indicate it fixes issues in v2 of Sam's
series. Changelogs in each patch.

Joel Stanley (1):
  config/aspeed: Enable NC-SI support

Samuel Mendoza-Jonas (2):
  net: NC-SI setup and handling
  net/ftgmac100: Add NC-SI mode support

 include/net.h                 |  2 +-
 include/phy.h                 |  2 ++
 cmd/net.c                     | 22 ++++++++++++++++++++++
 drivers/net/ftgmac100.c       | 14 ++++++++++----
 drivers/net/phy/ncsi.c        |  1 +
 drivers/net/phy/phy.c         |  9 ++++++++-
 net/net.c                     | 27 ++++++++++++++++++++++++++-
 cmd/Kconfig                   |  8 ++++++++
 configs/evb-ast2500_defconfig |  2 ++
 configs/evb-ast2600_defconfig |  2 ++
 10 files changed, 82 insertions(+), 7 deletions(-)

-- 
2.35.1

[PATCH v3 1/3] net: NC-SI setup and handling

[Joel Stanley]

From: Samuel Mendoza-Jonas <sam@mendozajonas.com>

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>
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
v3:
 - Fix compilation. There were no configs that enabled the NCSI phy code
   so it had bitrotted
 - Use NCSI_PHY instead of CMD_NCSI so NCSI can work without the command
 - Add phy_interface_is_ncsi() helper, thanks Cédric for this suggestion
 - Only create NCSI phy device when driver is configured for it

 include/net.h          |  2 +-
 include/phy.h          |  2 ++
 cmd/net.c              | 22 ++++++++++++++++++++++
 drivers/net/phy/ncsi.c |  1 +
 drivers/net/phy/phy.c  |  9 ++++++++-
 net/net.c              | 27 ++++++++++++++++++++++++++-
 cmd/Kconfig            |  8 ++++++++
 7 files changed, 68 insertions(+), 3 deletions(-)

diff --git a/include/net.h b/include/net.h
index e3889a0bc85e..0681b8246323 100644
--- a/include/net.h
+++ b/include/net.h
@@ -558,7 +558,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, UDP
+	TFTPSRV, TFTPPUT, LINKLOCAL, FASTBOOT, WOL, UDP, NCSI
 };
 
 extern char	net_boot_file_name[1024];/* Boot File name */
diff --git a/include/phy.h b/include/phy.h
index b32959571069..1e0f8856f629 100644
--- a/include/phy.h
+++ b/include/phy.h
@@ -583,6 +583,8 @@ static inline bool phy_interface_is_sgmii(struct phy_device *phydev)
 		phydev->interface <= PHY_INTERFACE_MODE_QSGMII;
 }
 
+bool phy_interface_is_ncsi(void);
+
 /* PHY UIDs for various PHYs that are referenced in external code */
 #define PHY_UID_CS4340		0x13e51002
 #define PHY_UID_CS4223		0x03e57003
diff --git a/cmd/net.c b/cmd/net.c
index 3619c843d838..2863fe768118 100644
--- a/cmd/net.c
+++ b/cmd/net.c
@@ -16,6 +16,7 @@
 #include <net.h>
 #include <net/udp.h>
 #include <net/sntp.h>
+#include <net/ncsi.h>
 
 static int netboot_common(enum proto_t, struct cmd_tbl *, int, char * const []);
 
@@ -524,3 +525,24 @@ U_BOOT_CMD(
 	"list - list available devices\n"
 );
 #endif // CONFIG_DM_ETH
+
+#if defined(CONFIG_CMD_NCSI)
+static int do_ncsi(struct cmd_tbl *cmdtp, int flag, int argc, char * const argv[])
+{
+	if (!phy_interface_is_ncsi() || !ncsi_active()) {
+		printf("Device not configured for NC-SI\n");
+		return CMD_RET_FAILURE;
+	}
+
+	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/drivers/net/phy/ncsi.c b/drivers/net/phy/ncsi.c
index bf1e832be9f1..bb7ecebed382 100644
--- a/drivers/net/phy/ncsi.c
+++ b/drivers/net/phy/ncsi.c
@@ -9,6 +9,7 @@
 #include <log.h>
 #include <malloc.h>
 #include <phy.h>
+#include <net.h>
 #include <net/ncsi.h>
 #include <net/ncsi-pkt.h>
 #include <asm/unaligned.h>
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 1121b99abff5..d04538838852 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -1026,7 +1026,7 @@ struct phy_device *phy_connect(struct mii_dev *bus, int addr,
 #endif
 
 #ifdef CONFIG_PHY_NCSI
-	if (!phydev)
+	if (!phydev && interface == PHY_INTERFACE_MODE_NCSI)
 		phydev = phy_device_create(bus, 0, PHY_NCSI_ID, false);
 #endif
 
@@ -1101,3 +1101,10 @@ int phy_modify(struct phy_device *phydev, int devad, int regnum, u16 mask,
 
 	return phy_write(phydev, devad, regnum, (ret & ~mask) | set);
 }
+
+bool phy_interface_is_ncsi(void)
+{
+	struct eth_pdata *pdata = dev_get_plat(eth_get_dev());
+
+	return pdata->phy_interface == PHY_INTERFACE_MODE_NCSI;
+}
diff --git a/net/net.c b/net/net.c
index 81905f631592..a4e645ac4425 100644
--- a/net/net.c
+++ b/net/net.c
@@ -93,6 +93,7 @@
 #include <net.h>
 #include <net/fastboot.h>
 #include <net/tftp.h>
+#include <net/ncsi.h>
 #if defined(CONFIG_CMD_PCAP)
 #include <net/pcap.h>
 #endif
@@ -410,6 +411,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 (phy_interface_is_ncsi() && 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()) {
@@ -423,6 +434,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_PHY_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", image_load_addr);
 			}
-			if (protocol != NETCONS)
+			if (protocol != NETCONS && protocol != NCSI)
 				eth_halt();
 			else
 				eth_halt_state_only();
@@ -1321,6 +1338,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
 	}
 }
@@ -1381,6 +1403,9 @@ common:
 
 #ifdef CONFIG_CMD_RARP
 	case RARP:
+#endif
+#ifdef CONFIG_PHY_NCSI
+	case NCSI:
 #endif
 	case BOOTP:
 	case CDP:
diff --git a/cmd/Kconfig b/cmd/Kconfig
index ad36ae63c659..bbd00707727d 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -1796,6 +1796,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
-- 
2.35.1

[PATCH v3 2/3] net/ftgmac100: Add NC-SI mode support

[Joel Stanley]

From: Samuel Mendoza-Jonas <sam@mendozajonas.com>

Update the ftgmac100 driver to support NC-SI instead of an mdio phy
where available. This is a common setup for Aspeed AST2x00 platforms.

NC-SI mode is determined from the device-tree if either phy-mode sets it
or the use-ncsi property exists. If set then normal mdio setup is
skipped in favour of the NC-SI phy.

Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
v3:
 - Simplify ncsi enable by re-using pdata->phy_interface parsing.
   use-ncsi still overrides this value.
 - Fix up freeing in remove callback per Joe's review

 drivers/net/ftgmac100.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ftgmac100.c b/drivers/net/ftgmac100.c
index 78779d7d60b9..69370ea5cca2 100644
--- a/drivers/net/ftgmac100.c
+++ b/drivers/net/ftgmac100.c
@@ -188,7 +188,7 @@ static int ftgmac100_phy_adjust_link(struct ftgmac100_data *priv)
 	struct phy_device *phydev = priv->phydev;
 	u32 maccr;
 
-	if (!phydev->link) {
+	if (!phydev->link && priv->phy_mode != PHY_INTERFACE_MODE_NCSI) {
 		dev_err(phydev->dev, "No link\n");
 		return -EREMOTEIO;
 	}
@@ -228,7 +228,8 @@ static int ftgmac100_phy_init(struct udevice *dev)
 	if (!phydev)
 		return -ENODEV;
 
-	phydev->supported &= PHY_GBIT_FEATURES;
+	if (priv->phy_mode != PHY_INTERFACE_MODE_NCSI)
+		phydev->supported &= PHY_GBIT_FEATURES;
 	if (priv->max_speed) {
 		ret = phy_set_supported(phydev, priv->max_speed);
 		if (ret)
@@ -308,7 +309,8 @@ static void ftgmac100_stop(struct udevice *dev)
 
 	writel(0, &ftgmac100->maccr);
 
-	phy_shutdown(priv->phydev);
+	if (priv->phy_mode != PHY_INTERFACE_MODE_NCSI)
+		phy_shutdown(priv->phydev);
 }
 
 static int ftgmac100_start(struct udevice *dev)
@@ -580,6 +582,9 @@ static int ftgmac100_probe(struct udevice *dev)
 	priv->max_speed = pdata->max_speed;
 	priv->phy_addr = 0;
 
+	if (dev_read_bool(dev, "use-ncsi"))
+		priv->phy_mode = PHY_INTERFACE_MODE_NCSI;
+
 #ifdef CONFIG_PHY_ADDR
 	priv->phy_addr = CONFIG_PHY_ADDR;
 #endif
@@ -592,7 +597,8 @@ static int ftgmac100_probe(struct udevice *dev)
 	 * If DM MDIO is enabled, the MDIO bus will be initialized later in
 	 * dm_eth_phy_connect
 	 */
-	if (!IS_ENABLED(CONFIG_DM_MDIO)) {
+	if (priv->phy_mode != PHY_INTERFACE_MODE_NCSI &&
+	    !IS_ENABLED(CONFIG_DM_MDIO)) {
 		ret = ftgmac100_mdio_init(dev);
 		if (ret) {
 			dev_err(dev, "Failed to initialize mdiobus: %d\n", ret);
-- 
2.35.1

[PATCH v3 3/3] config/aspeed: Enable NC-SI support

[Joel Stanley]

Aspeed BMCs are commonly used with NC-SI. A system indicates the driver
should configure the link over NC-SI using the device tree.

Add it to the defconfig so we get compile coverage of the driver, even
if the EVBs do not normally use it.

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 configs/evb-ast2500_defconfig | 2 ++
 configs/evb-ast2600_defconfig | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/configs/evb-ast2500_defconfig b/configs/evb-ast2500_defconfig
index 9d2c4f81c5ad..866732117aa7 100644
--- a/configs/evb-ast2500_defconfig
+++ b/configs/evb-ast2500_defconfig
@@ -27,6 +27,7 @@ CONFIG_CMD_DHCP=y
 CONFIG_BOOTP_BOOTFILESIZE=y
 CONFIG_CMD_MII=y
 CONFIG_CMD_PING=y
+CONFIG_CMD_NCSI=y
 CONFIG_ENV_OVERWRITE=y
 CONFIG_SYS_RELOC_GD_ENV_ADDR=y
 CONFIG_NET_RANDOM_ETHADDR=y
@@ -38,6 +39,7 @@ CONFIG_SYS_I2C_ASPEED=y
 CONFIG_MMC_SDHCI=y
 CONFIG_MMC_SDHCI_ASPEED=y
 CONFIG_PHY_REALTEK=y
+CONFIG_PHY_NCSI=y
 CONFIG_DM_ETH=y
 CONFIG_FTGMAC100=y
 CONFIG_PHY=y
diff --git a/configs/evb-ast2600_defconfig b/configs/evb-ast2600_defconfig
index 5c4d8426607c..8be22014bb4e 100644
--- a/configs/evb-ast2600_defconfig
+++ b/configs/evb-ast2600_defconfig
@@ -54,6 +54,7 @@ CONFIG_CMD_DHCP=y
 CONFIG_BOOTP_BOOTFILESIZE=y
 CONFIG_CMD_MII=y
 CONFIG_CMD_PING=y
+CONFIG_CMD_NCSI=y
 CONFIG_SPL_OF_CONTROL=y
 CONFIG_ENV_OVERWRITE=y
 CONFIG_SYS_RELOC_GD_ENV_ADDR=y
@@ -73,6 +74,7 @@ CONFIG_MISC=y
 CONFIG_MMC_SDHCI=y
 CONFIG_MMC_SDHCI_ASPEED=y
 CONFIG_PHY_REALTEK=y
+CONFIG_PHY_NCSI=y
 CONFIG_DM_ETH=y
 CONFIG_DM_MDIO=y
 CONFIG_FTGMAC100=y
-- 
2.35.1

Re: [PATCH v3 2/3] net/ftgmac100: Add NC-SI mode support

[Cédric Le Goater]

On 7/4/22 09:58, Joel Stanley wrote:
> From: Samuel Mendoza-Jonas <sam@mendozajonas.com>
> 
> Update the ftgmac100 driver to support NC-SI instead of an mdio phy
> where available. This is a common setup for Aspeed AST2x00 platforms.
> 
> NC-SI mode is determined from the device-tree if either phy-mode sets it
> or the use-ncsi property exists. If set then normal mdio setup is
> skipped in favour of the NC-SI phy.
> 
> Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>
> Signed-off-by: Joel Stanley <joel@jms.id.au>

Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.


> ---
> v3:
>   - Simplify ncsi enable by re-using pdata->phy_interface parsing.
>     use-ncsi still overrides this value.
>   - Fix up freeing in remove callback per Joe's review
> 
>   drivers/net/ftgmac100.c | 14 ++++++++++----
>   1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ftgmac100.c b/drivers/net/ftgmac100.c
> index 78779d7d60b9..69370ea5cca2 100644
> --- a/drivers/net/ftgmac100.c
> +++ b/drivers/net/ftgmac100.c
> @@ -188,7 +188,7 @@ static int ftgmac100_phy_adjust_link(struct ftgmac100_data *priv)
>   	struct phy_device *phydev = priv->phydev;
>   	u32 maccr;
>   
> -	if (!phydev->link) {
> +	if (!phydev->link && priv->phy_mode != PHY_INTERFACE_MODE_NCSI) {
>   		dev_err(phydev->dev, "No link\n");
>   		return -EREMOTEIO;
>   	}
> @@ -228,7 +228,8 @@ static int ftgmac100_phy_init(struct udevice *dev)
>   	if (!phydev)
>   		return -ENODEV;
>   
> -	phydev->supported &= PHY_GBIT_FEATURES;
> +	if (priv->phy_mode != PHY_INTERFACE_MODE_NCSI)
> +		phydev->supported &= PHY_GBIT_FEATURES;
>   	if (priv->max_speed) {
>   		ret = phy_set_supported(phydev, priv->max_speed);
>   		if (ret)
> @@ -308,7 +309,8 @@ static void ftgmac100_stop(struct udevice *dev)
>   
>   	writel(0, &ftgmac100->maccr);
>   
> -	phy_shutdown(priv->phydev);
> +	if (priv->phy_mode != PHY_INTERFACE_MODE_NCSI)
> +		phy_shutdown(priv->phydev);
>   }
>   
>   static int ftgmac100_start(struct udevice *dev)
> @@ -580,6 +582,9 @@ static int ftgmac100_probe(struct udevice *dev)
>   	priv->max_speed = pdata->max_speed;
>   	priv->phy_addr = 0;
>   
> +	if (dev_read_bool(dev, "use-ncsi"))
> +		priv->phy_mode = PHY_INTERFACE_MODE_NCSI;
> +
>   #ifdef CONFIG_PHY_ADDR
>   	priv->phy_addr = CONFIG_PHY_ADDR;
>   #endif
> @@ -592,7 +597,8 @@ static int ftgmac100_probe(struct udevice *dev)
>   	 * If DM MDIO is enabled, the MDIO bus will be initialized later in
>   	 * dm_eth_phy_connect
>   	 */
> -	if (!IS_ENABLED(CONFIG_DM_MDIO)) {
> +	if (priv->phy_mode != PHY_INTERFACE_MODE_NCSI &&
> +	    !IS_ENABLED(CONFIG_DM_MDIO)) {
>   		ret = ftgmac100_mdio_init(dev);
>   		if (ret) {
>   			dev_err(dev, "Failed to initialize mdiobus: %d\n", ret);

Re: [PATCH v3 1/3] net: NC-SI setup and handling

[Cédric Le Goater]

On 7/4/22 09:58, Joel Stanley wrote:
> From: Samuel Mendoza-Jonas <sam@mendozajonas.com>
> 
> 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>
> Signed-off-by: Joel Stanley <joel@jms.id.au>


Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

> ---
> v3:
>   - Fix compilation. There were no configs that enabled the NCSI phy code
>     so it had bitrotted
>   - Use NCSI_PHY instead of CMD_NCSI so NCSI can work without the command
>   - Add phy_interface_is_ncsi() helper, thanks Cédric for this suggestion
>   - Only create NCSI phy device when driver is configured for it
> 
>   include/net.h          |  2 +-
>   include/phy.h          |  2 ++
>   cmd/net.c              | 22 ++++++++++++++++++++++
>   drivers/net/phy/ncsi.c |  1 +
>   drivers/net/phy/phy.c  |  9 ++++++++-
>   net/net.c              | 27 ++++++++++++++++++++++++++-
>   cmd/Kconfig            |  8 ++++++++
>   7 files changed, 68 insertions(+), 3 deletions(-)
> 
> diff --git a/include/net.h b/include/net.h
> index e3889a0bc85e..0681b8246323 100644
> --- a/include/net.h
> +++ b/include/net.h
> @@ -558,7 +558,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, UDP
> +	TFTPSRV, TFTPPUT, LINKLOCAL, FASTBOOT, WOL, UDP, NCSI
>   };
>   
>   extern char	net_boot_file_name[1024];/* Boot File name */
> diff --git a/include/phy.h b/include/phy.h
> index b32959571069..1e0f8856f629 100644
> --- a/include/phy.h
> +++ b/include/phy.h
> @@ -583,6 +583,8 @@ static inline bool phy_interface_is_sgmii(struct phy_device *phydev)
>   		phydev->interface <= PHY_INTERFACE_MODE_QSGMII;
>   }
>   
> +bool phy_interface_is_ncsi(void);
> +
>   /* PHY UIDs for various PHYs that are referenced in external code */
>   #define PHY_UID_CS4340		0x13e51002
>   #define PHY_UID_CS4223		0x03e57003
> diff --git a/cmd/net.c b/cmd/net.c
> index 3619c843d838..2863fe768118 100644
> --- a/cmd/net.c
> +++ b/cmd/net.c
> @@ -16,6 +16,7 @@
>   #include <net.h>
>   #include <net/udp.h>
>   #include <net/sntp.h>
> +#include <net/ncsi.h>
>   
>   static int netboot_common(enum proto_t, struct cmd_tbl *, int, char * const []);
>   
> @@ -524,3 +525,24 @@ U_BOOT_CMD(
>   	"list - list available devices\n"
>   );
>   #endif // CONFIG_DM_ETH
> +
> +#if defined(CONFIG_CMD_NCSI)
> +static int do_ncsi(struct cmd_tbl *cmdtp, int flag, int argc, char * const argv[])
> +{
> +	if (!phy_interface_is_ncsi() || !ncsi_active()) {
> +		printf("Device not configured for NC-SI\n");
> +		return CMD_RET_FAILURE;
> +	}
> +
> +	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/drivers/net/phy/ncsi.c b/drivers/net/phy/ncsi.c
> index bf1e832be9f1..bb7ecebed382 100644
> --- a/drivers/net/phy/ncsi.c
> +++ b/drivers/net/phy/ncsi.c
> @@ -9,6 +9,7 @@
>   #include <log.h>
>   #include <malloc.h>
>   #include <phy.h>
> +#include <net.h>
>   #include <net/ncsi.h>
>   #include <net/ncsi-pkt.h>
>   #include <asm/unaligned.h>
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index 1121b99abff5..d04538838852 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -1026,7 +1026,7 @@ struct phy_device *phy_connect(struct mii_dev *bus, int addr,
>   #endif
>   
>   #ifdef CONFIG_PHY_NCSI
> -	if (!phydev)
> +	if (!phydev && interface == PHY_INTERFACE_MODE_NCSI)
>   		phydev = phy_device_create(bus, 0, PHY_NCSI_ID, false);
>   #endif
>   
> @@ -1101,3 +1101,10 @@ int phy_modify(struct phy_device *phydev, int devad, int regnum, u16 mask,
>   
>   	return phy_write(phydev, devad, regnum, (ret & ~mask) | set);
>   }
> +
> +bool phy_interface_is_ncsi(void)
> +{
> +	struct eth_pdata *pdata = dev_get_plat(eth_get_dev());
> +
> +	return pdata->phy_interface == PHY_INTERFACE_MODE_NCSI;
> +}
> diff --git a/net/net.c b/net/net.c
> index 81905f631592..a4e645ac4425 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -93,6 +93,7 @@
>   #include <net.h>
>   #include <net/fastboot.h>
>   #include <net/tftp.h>
> +#include <net/ncsi.h>
>   #if defined(CONFIG_CMD_PCAP)
>   #include <net/pcap.h>
>   #endif
> @@ -410,6 +411,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 (phy_interface_is_ncsi() && 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()) {
> @@ -423,6 +434,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_PHY_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", image_load_addr);
>   			}
> -			if (protocol != NETCONS)
> +			if (protocol != NETCONS && protocol != NCSI)
>   				eth_halt();
>   			else
>   				eth_halt_state_only();
> @@ -1321,6 +1338,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
>   	}
>   }
> @@ -1381,6 +1403,9 @@ common:
>   
>   #ifdef CONFIG_CMD_RARP
>   	case RARP:
> +#endif
> +#ifdef CONFIG_PHY_NCSI
> +	case NCSI:
>   #endif
>   	case BOOTP:
>   	case CDP:
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index ad36ae63c659..bbd00707727d 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -1796,6 +1796,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

Re: [PATCH v3 3/3] config/aspeed: Enable NC-SI support

[Cédric Le Goater]

On 7/4/22 09:58, Joel Stanley wrote:
> Aspeed BMCs are commonly used with NC-SI. A system indicates the driver
> should configure the link over NC-SI using the device tree.
> 
> Add it to the defconfig so we get compile coverage of the driver, even
> if the EVBs do not normally use it.
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>

Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

> ---
>   configs/evb-ast2500_defconfig | 2 ++
>   configs/evb-ast2600_defconfig | 2 ++
>   2 files changed, 4 insertions(+)
> 
> diff --git a/configs/evb-ast2500_defconfig b/configs/evb-ast2500_defconfig
> index 9d2c4f81c5ad..866732117aa7 100644
> --- a/configs/evb-ast2500_defconfig
> +++ b/configs/evb-ast2500_defconfig
> @@ -27,6 +27,7 @@ CONFIG_CMD_DHCP=y
>   CONFIG_BOOTP_BOOTFILESIZE=y
>   CONFIG_CMD_MII=y
>   CONFIG_CMD_PING=y
> +CONFIG_CMD_NCSI=y
>   CONFIG_ENV_OVERWRITE=y
>   CONFIG_SYS_RELOC_GD_ENV_ADDR=y
>   CONFIG_NET_RANDOM_ETHADDR=y
> @@ -38,6 +39,7 @@ CONFIG_SYS_I2C_ASPEED=y
>   CONFIG_MMC_SDHCI=y
>   CONFIG_MMC_SDHCI_ASPEED=y
>   CONFIG_PHY_REALTEK=y
> +CONFIG_PHY_NCSI=y
>   CONFIG_DM_ETH=y
>   CONFIG_FTGMAC100=y
>   CONFIG_PHY=y
> diff --git a/configs/evb-ast2600_defconfig b/configs/evb-ast2600_defconfig
> index 5c4d8426607c..8be22014bb4e 100644
> --- a/configs/evb-ast2600_defconfig
> +++ b/configs/evb-ast2600_defconfig
> @@ -54,6 +54,7 @@ CONFIG_CMD_DHCP=y
>   CONFIG_BOOTP_BOOTFILESIZE=y
>   CONFIG_CMD_MII=y
>   CONFIG_CMD_PING=y
> +CONFIG_CMD_NCSI=y
>   CONFIG_SPL_OF_CONTROL=y
>   CONFIG_ENV_OVERWRITE=y
>   CONFIG_SYS_RELOC_GD_ENV_ADDR=y
> @@ -73,6 +74,7 @@ CONFIG_MISC=y
>   CONFIG_MMC_SDHCI=y
>   CONFIG_MMC_SDHCI_ASPEED=y
>   CONFIG_PHY_REALTEK=y
> +CONFIG_PHY_NCSI=y
>   CONFIG_DM_ETH=y
>   CONFIG_DM_MDIO=y
>   CONFIG_FTGMAC100=y

Re: [PATCH v3 0/3] Enable NC-SI support

[Joel Stanley]

On Mon, 4 Jul 2022 at 07:58, Joel Stanley <joel@jms.id.au> wrote:
>
> Back in 2019 Sam submitted NC-SI support. The NC-SI PHY driver was
> merged (patches 1 and 2), but we never got around to merging patches 3
> and 4:
>
>  https://lore.kernel.org/u-boot/20190618013720.2823-1-sam@mendozajonas.com/
>
> Sam as long since moved on from working on the Aspeed BMCs, but the code
> has been in use in the vendor fork for some time.
>
> This refreshes his patches and enables support in the Aspeed defconfigs,
> giving compile coverage to the NC-SI phy.
>
> I've called the series v3 to indicate it fixes issues in v2 of Sam's
> series. Changelogs in each patch.
>
> Joel Stanley (1):
>   config/aspeed: Enable NC-SI support
>
> Samuel Mendoza-Jonas (2):
>   net: NC-SI setup and handling
>   net/ftgmac100: Add NC-SI mode support

Hello Tom, Ramon, Joe,

These ones have some reviews from Cedric. I'm not familiar with the
u-boot process and which tree they get merged into. Can you help me
out?

Cheers,

Joel


>
>  include/net.h                 |  2 +-
>  include/phy.h                 |  2 ++
>  cmd/net.c                     | 22 ++++++++++++++++++++++
>  drivers/net/ftgmac100.c       | 14 ++++++++++----
>  drivers/net/phy/ncsi.c        |  1 +
>  drivers/net/phy/phy.c         |  9 ++++++++-
>  net/net.c                     | 27 ++++++++++++++++++++++++++-
>  cmd/Kconfig                   |  8 ++++++++
>  configs/evb-ast2500_defconfig |  2 ++
>  configs/evb-ast2600_defconfig |  2 ++
>  10 files changed, 82 insertions(+), 7 deletions(-)
>
> --
> 2.35.1
>

Re: [PATCH v3 2/3] net/ftgmac100: Add NC-SI mode support

[Ramon Fried]

On Mon, Jul 4, 2022 at 11:20 AM Cédric Le Goater <clg@kaod.org> wrote:
>
> On 7/4/22 09:58, Joel Stanley wrote:
> > From: Samuel Mendoza-Jonas <sam@mendozajonas.com>
> >
> > Update the ftgmac100 driver to support NC-SI instead of an mdio phy
> > where available. This is a common setup for Aspeed AST2x00 platforms.
> >
> > NC-SI mode is determined from the device-tree if either phy-mode sets it
> > or the use-ncsi property exists. If set then normal mdio setup is
> > skipped in favour of the NC-SI phy.
> >
> > Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>
> > Signed-off-by: Joel Stanley <joel@jms.id.au>
>
> Reviewed-by: Cédric Le Goater <clg@kaod.org>
>
> Thanks,
>
> C.
>
>
> > ---
> > v3:
> >   - Simplify ncsi enable by re-using pdata->phy_interface parsing.
> >     use-ncsi still overrides this value.
> >   - Fix up freeing in remove callback per Joe's review
> >
> >   drivers/net/ftgmac100.c | 14 ++++++++++----
> >   1 file changed, 10 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/ftgmac100.c b/drivers/net/ftgmac100.c
> > index 78779d7d60b9..69370ea5cca2 100644
> > --- a/drivers/net/ftgmac100.c
> > +++ b/drivers/net/ftgmac100.c
> > @@ -188,7 +188,7 @@ static int ftgmac100_phy_adjust_link(struct ftgmac100_data *priv)
> >       struct phy_device *phydev = priv->phydev;
> >       u32 maccr;
> >
> > -     if (!phydev->link) {
> > +     if (!phydev->link && priv->phy_mode != PHY_INTERFACE_MODE_NCSI) {
> >               dev_err(phydev->dev, "No link\n");
> >               return -EREMOTEIO;
> >       }
> > @@ -228,7 +228,8 @@ static int ftgmac100_phy_init(struct udevice *dev)
> >       if (!phydev)
> >               return -ENODEV;
> >
> > -     phydev->supported &= PHY_GBIT_FEATURES;
> > +     if (priv->phy_mode != PHY_INTERFACE_MODE_NCSI)
> > +             phydev->supported &= PHY_GBIT_FEATURES;
> >       if (priv->max_speed) {
> >               ret = phy_set_supported(phydev, priv->max_speed);
> >               if (ret)
> > @@ -308,7 +309,8 @@ static void ftgmac100_stop(struct udevice *dev)
> >
> >       writel(0, &ftgmac100->maccr);
> >
> > -     phy_shutdown(priv->phydev);
> > +     if (priv->phy_mode != PHY_INTERFACE_MODE_NCSI)
> > +             phy_shutdown(priv->phydev);
> >   }
> >
> >   static int ftgmac100_start(struct udevice *dev)
> > @@ -580,6 +582,9 @@ static int ftgmac100_probe(struct udevice *dev)
> >       priv->max_speed = pdata->max_speed;
> >       priv->phy_addr = 0;
> >
> > +     if (dev_read_bool(dev, "use-ncsi"))
> > +             priv->phy_mode = PHY_INTERFACE_MODE_NCSI;
> > +
> >   #ifdef CONFIG_PHY_ADDR
> >       priv->phy_addr = CONFIG_PHY_ADDR;
> >   #endif
> > @@ -592,7 +597,8 @@ static int ftgmac100_probe(struct udevice *dev)
> >        * If DM MDIO is enabled, the MDIO bus will be initialized later in
> >        * dm_eth_phy_connect
> >        */
> > -     if (!IS_ENABLED(CONFIG_DM_MDIO)) {
> > +     if (priv->phy_mode != PHY_INTERFACE_MODE_NCSI &&
> > +         !IS_ENABLED(CONFIG_DM_MDIO)) {
> >               ret = ftgmac100_mdio_init(dev);
> >               if (ret) {
> >                       dev_err(dev, "Failed to initialize mdiobus: %d\n", ret);
>
Reviewed-by: Ramon Fried <rfried.dev@gmail.com>

Re: [PATCH v3 0/3] Enable NC-SI support

[Ramon Fried]

On Fri, Aug 5, 2022 at 9:32 AM Joel Stanley <joel@jms.id.au> wrote:
>
> On Mon, 4 Jul 2022 at 07:58, Joel Stanley <joel@jms.id.au> wrote:
> >
> > Back in 2019 Sam submitted NC-SI support. The NC-SI PHY driver was
> > merged (patches 1 and 2), but we never got around to merging patches 3
> > and 4:
> >
> >  https://lore.kernel.org/u-boot/20190618013720.2823-1-sam@mendozajonas.com/
> >
> > Sam as long since moved on from working on the Aspeed BMCs, but the code
> > has been in use in the vendor fork for some time.
> >
> > This refreshes his patches and enables support in the Aspeed defconfigs,
> > giving compile coverage to the NC-SI phy.
> >
> > I've called the series v3 to indicate it fixes issues in v2 of Sam's
> > series. Changelogs in each patch.
> >
> > Joel Stanley (1):
> >   config/aspeed: Enable NC-SI support
> >
> > Samuel Mendoza-Jonas (2):
> >   net: NC-SI setup and handling
> >   net/ftgmac100: Add NC-SI mode support
>
> Hello Tom, Ramon, Joe,
>
> These ones have some reviews from Cedric. I'm not familiar with the
> u-boot process and which tree they get merged into. Can you help me
> out?
>
> Cheers,
>
> Joel
>
>
> >
> >  include/net.h                 |  2 +-
> >  include/phy.h                 |  2 ++
> >  cmd/net.c                     | 22 ++++++++++++++++++++++
> >  drivers/net/ftgmac100.c       | 14 ++++++++++----
> >  drivers/net/phy/ncsi.c        |  1 +
> >  drivers/net/phy/phy.c         |  9 ++++++++-
> >  net/net.c                     | 27 ++++++++++++++++++++++++++-
> >  cmd/Kconfig                   |  8 ++++++++
> >  configs/evb-ast2500_defconfig |  2 ++
> >  configs/evb-ast2600_defconfig |  2 ++
> >  10 files changed, 82 insertions(+), 7 deletions(-)
> >
> > --
> > 2.35.1
> >
Sure, I'm on it.

Re: [PATCH v3 1/3] net: NC-SI setup and handling

[Ramon Fried]

On Mon, Jul 4, 2022 at 11:20 AM Cédric Le Goater <clg@kaod.org> wrote:
>
> On 7/4/22 09:58, Joel Stanley wrote:
> > From: Samuel Mendoza-Jonas <sam@mendozajonas.com>
> >
> > 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>
> > Signed-off-by: Joel Stanley <joel@jms.id.au>
>
>
> Reviewed-by: Cédric Le Goater <clg@kaod.org>
>
> Thanks,
>
> C.
>
> > ---
> > v3:
> >   - Fix compilation. There were no configs that enabled the NCSI phy code
> >     so it had bitrotted
> >   - Use NCSI_PHY instead of CMD_NCSI so NCSI can work without the command
> >   - Add phy_interface_is_ncsi() helper, thanks Cédric for this suggestion
> >   - Only create NCSI phy device when driver is configured for it
> >
> >   include/net.h          |  2 +-
> >   include/phy.h          |  2 ++
> >   cmd/net.c              | 22 ++++++++++++++++++++++
> >   drivers/net/phy/ncsi.c |  1 +
> >   drivers/net/phy/phy.c  |  9 ++++++++-
> >   net/net.c              | 27 ++++++++++++++++++++++++++-
> >   cmd/Kconfig            |  8 ++++++++
> >   7 files changed, 68 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/net.h b/include/net.h
> > index e3889a0bc85e..0681b8246323 100644
> > --- a/include/net.h
> > +++ b/include/net.h
> > @@ -558,7 +558,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, UDP
> > +     TFTPSRV, TFTPPUT, LINKLOCAL, FASTBOOT, WOL, UDP, NCSI
> >   };
> >
> >   extern char net_boot_file_name[1024];/* Boot File name */
> > diff --git a/include/phy.h b/include/phy.h
> > index b32959571069..1e0f8856f629 100644
> > --- a/include/phy.h
> > +++ b/include/phy.h
> > @@ -583,6 +583,8 @@ static inline bool phy_interface_is_sgmii(struct phy_device *phydev)
> >               phydev->interface <= PHY_INTERFACE_MODE_QSGMII;
> >   }
> >
> > +bool phy_interface_is_ncsi(void);
> > +
> >   /* PHY UIDs for various PHYs that are referenced in external code */
> >   #define PHY_UID_CS4340              0x13e51002
> >   #define PHY_UID_CS4223              0x03e57003
> > diff --git a/cmd/net.c b/cmd/net.c
> > index 3619c843d838..2863fe768118 100644
> > --- a/cmd/net.c
> > +++ b/cmd/net.c
> > @@ -16,6 +16,7 @@
> >   #include <net.h>
> >   #include <net/udp.h>
> >   #include <net/sntp.h>
> > +#include <net/ncsi.h>
> >
> >   static int netboot_common(enum proto_t, struct cmd_tbl *, int, char * const []);
> >
> > @@ -524,3 +525,24 @@ U_BOOT_CMD(
> >       "list - list available devices\n"
> >   );
> >   #endif // CONFIG_DM_ETH
> > +
> > +#if defined(CONFIG_CMD_NCSI)
> > +static int do_ncsi(struct cmd_tbl *cmdtp, int flag, int argc, char * const argv[])
> > +{
> > +     if (!phy_interface_is_ncsi() || !ncsi_active()) {
> > +             printf("Device not configured for NC-SI\n");
> > +             return CMD_RET_FAILURE;
> > +     }
> > +
> > +     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/drivers/net/phy/ncsi.c b/drivers/net/phy/ncsi.c
> > index bf1e832be9f1..bb7ecebed382 100644
> > --- a/drivers/net/phy/ncsi.c
> > +++ b/drivers/net/phy/ncsi.c
> > @@ -9,6 +9,7 @@
> >   #include <log.h>
> >   #include <malloc.h>
> >   #include <phy.h>
> > +#include <net.h>
> >   #include <net/ncsi.h>
> >   #include <net/ncsi-pkt.h>
> >   #include <asm/unaligned.h>
> > diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> > index 1121b99abff5..d04538838852 100644
> > --- a/drivers/net/phy/phy.c
> > +++ b/drivers/net/phy/phy.c
> > @@ -1026,7 +1026,7 @@ struct phy_device *phy_connect(struct mii_dev *bus, int addr,
> >   #endif
> >
> >   #ifdef CONFIG_PHY_NCSI
> > -     if (!phydev)
> > +     if (!phydev && interface == PHY_INTERFACE_MODE_NCSI)
> >               phydev = phy_device_create(bus, 0, PHY_NCSI_ID, false);
> >   #endif
> >
> > @@ -1101,3 +1101,10 @@ int phy_modify(struct phy_device *phydev, int devad, int regnum, u16 mask,
> >
> >       return phy_write(phydev, devad, regnum, (ret & ~mask) | set);
> >   }
> > +
> > +bool phy_interface_is_ncsi(void)
> > +{
> > +     struct eth_pdata *pdata = dev_get_plat(eth_get_dev());
> > +
> > +     return pdata->phy_interface == PHY_INTERFACE_MODE_NCSI;
> > +}
> > diff --git a/net/net.c b/net/net.c
> > index 81905f631592..a4e645ac4425 100644
> > --- a/net/net.c
> > +++ b/net/net.c
> > @@ -93,6 +93,7 @@
> >   #include <net.h>
> >   #include <net/fastboot.h>
> >   #include <net/tftp.h>
> > +#include <net/ncsi.h>
> >   #if defined(CONFIG_CMD_PCAP)
> >   #include <net/pcap.h>
> >   #endif
> > @@ -410,6 +411,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 (phy_interface_is_ncsi() && 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()) {
> > @@ -423,6 +434,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_PHY_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", image_load_addr);
> >                       }
> > -                     if (protocol != NETCONS)
> > +                     if (protocol != NETCONS && protocol != NCSI)
> >                               eth_halt();
> >                       else
> >                               eth_halt_state_only();
> > @@ -1321,6 +1338,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
> >       }
> >   }
> > @@ -1381,6 +1403,9 @@ common:
> >
> >   #ifdef CONFIG_CMD_RARP
> >       case RARP:
> > +#endif
> > +#ifdef CONFIG_PHY_NCSI
> > +     case NCSI:
> >   #endif
> >       case BOOTP:
> >       case CDP:
> > diff --git a/cmd/Kconfig b/cmd/Kconfig
> > index ad36ae63c659..bbd00707727d 100644
> > --- a/cmd/Kconfig
> > +++ b/cmd/Kconfig
> > @@ -1796,6 +1796,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
>
Please separate the command from the ncsi generic code.
The command interface should be an option that depends on ncsi support.

Re: [PATCH v3 3/3] config/aspeed: Enable NC-SI support

[Ramon Fried]

On Mon, Jul 4, 2022 at 11:21 AM Cédric Le Goater <clg@kaod.org> wrote:
>
> On 7/4/22 09:58, Joel Stanley wrote:
> > Aspeed BMCs are commonly used with NC-SI. A system indicates the driver
> > should configure the link over NC-SI using the device tree.
> >
> > Add it to the defconfig so we get compile coverage of the driver, even
> > if the EVBs do not normally use it.
> >
> > Signed-off-by: Joel Stanley <joel@jms.id.au>
>
> Reviewed-by: Cédric Le Goater <clg@kaod.org>
>
> Thanks,
>
> C.
>
> > ---
> >   configs/evb-ast2500_defconfig | 2 ++
> >   configs/evb-ast2600_defconfig | 2 ++
> >   2 files changed, 4 insertions(+)
> >
> > diff --git a/configs/evb-ast2500_defconfig b/configs/evb-ast2500_defconfig
> > index 9d2c4f81c5ad..866732117aa7 100644
> > --- a/configs/evb-ast2500_defconfig
> > +++ b/configs/evb-ast2500_defconfig
> > @@ -27,6 +27,7 @@ CONFIG_CMD_DHCP=y
> >   CONFIG_BOOTP_BOOTFILESIZE=y
> >   CONFIG_CMD_MII=y
> >   CONFIG_CMD_PING=y
> > +CONFIG_CMD_NCSI=y
> >   CONFIG_ENV_OVERWRITE=y
> >   CONFIG_SYS_RELOC_GD_ENV_ADDR=y
> >   CONFIG_NET_RANDOM_ETHADDR=y
> > @@ -38,6 +39,7 @@ CONFIG_SYS_I2C_ASPEED=y
> >   CONFIG_MMC_SDHCI=y
> >   CONFIG_MMC_SDHCI_ASPEED=y
> >   CONFIG_PHY_REALTEK=y
> > +CONFIG_PHY_NCSI=y
> >   CONFIG_DM_ETH=y
> >   CONFIG_FTGMAC100=y
> >   CONFIG_PHY=y
> > diff --git a/configs/evb-ast2600_defconfig b/configs/evb-ast2600_defconfig
> > index 5c4d8426607c..8be22014bb4e 100644
> > --- a/configs/evb-ast2600_defconfig
> > +++ b/configs/evb-ast2600_defconfig
> > @@ -54,6 +54,7 @@ CONFIG_CMD_DHCP=y
> >   CONFIG_BOOTP_BOOTFILESIZE=y
> >   CONFIG_CMD_MII=y
> >   CONFIG_CMD_PING=y
> > +CONFIG_CMD_NCSI=y
> >   CONFIG_SPL_OF_CONTROL=y
> >   CONFIG_ENV_OVERWRITE=y
> >   CONFIG_SYS_RELOC_GD_ENV_ADDR=y
> > @@ -73,6 +74,7 @@ CONFIG_MISC=y
> >   CONFIG_MMC_SDHCI=y
> >   CONFIG_MMC_SDHCI_ASPEED=y
> >   CONFIG_PHY_REALTEK=y
> > +CONFIG_PHY_NCSI=y
> >   CONFIG_DM_ETH=y
> >   CONFIG_DM_MDIO=y
> >   CONFIG_FTGMAC100=y
>
Reviewed-by: Ramon Fried <rfried.dev@gmail.com>

Re: [PATCH v3 1/3] net: NC-SI setup and handling

[Joel Stanley]

On Sat, 6 Aug 2022 at 23:54, Ramon Fried <rfried.dev@gmail.com> wrote:
>
> On Mon, Jul 4, 2022 at 11:20 AM Cédric Le Goater <clg@kaod.org> wrote:
> >
> > On 7/4/22 09:58, Joel Stanley wrote:
> > > From: Samuel Mendoza-Jonas <sam@mendozajonas.com>
> > >
> > > 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>
> > > Signed-off-by: Joel Stanley <joel@jms.id.au>
> >
> >
> > Reviewed-by: Cédric Le Goater <clg@kaod.org>
> >
> > Thanks,
> >
> > C.
> >
> > > ---
> > > v3:
> > >   - Fix compilation. There were no configs that enabled the NCSI phy code
> > >     so it had bitrotted
> > >   - Use NCSI_PHY instead of CMD_NCSI so NCSI can work without the command
> > >   - Add phy_interface_is_ncsi() helper, thanks Cédric for this suggestion
> > >   - Only create NCSI phy device when driver is configured for it
> > >
> > >   include/net.h          |  2 +-
> > >   include/phy.h          |  2 ++
> > >   cmd/net.c              | 22 ++++++++++++++++++++++
> > >   drivers/net/phy/ncsi.c |  1 +
> > >   drivers/net/phy/phy.c  |  9 ++++++++-
> > >   net/net.c              | 27 ++++++++++++++++++++++++++-
> > >   cmd/Kconfig            |  8 ++++++++
> > >   7 files changed, 68 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/include/net.h b/include/net.h
> > > index e3889a0bc85e..0681b8246323 100644
> > > --- a/include/net.h
> > > +++ b/include/net.h
> > > @@ -558,7 +558,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, UDP
> > > +     TFTPSRV, TFTPPUT, LINKLOCAL, FASTBOOT, WOL, UDP, NCSI
> > >   };
> > >
> > >   extern char net_boot_file_name[1024];/* Boot File name */
> > > diff --git a/include/phy.h b/include/phy.h
> > > index b32959571069..1e0f8856f629 100644
> > > --- a/include/phy.h
> > > +++ b/include/phy.h
> > > @@ -583,6 +583,8 @@ static inline bool phy_interface_is_sgmii(struct phy_device *phydev)
> > >               phydev->interface <= PHY_INTERFACE_MODE_QSGMII;
> > >   }
> > >
> > > +bool phy_interface_is_ncsi(void);
> > > +
> > >   /* PHY UIDs for various PHYs that are referenced in external code */
> > >   #define PHY_UID_CS4340              0x13e51002
> > >   #define PHY_UID_CS4223              0x03e57003
> > > diff --git a/cmd/net.c b/cmd/net.c
> > > index 3619c843d838..2863fe768118 100644
> > > --- a/cmd/net.c
> > > +++ b/cmd/net.c
> > > @@ -16,6 +16,7 @@
> > >   #include <net.h>
> > >   #include <net/udp.h>
> > >   #include <net/sntp.h>
> > > +#include <net/ncsi.h>
> > >
> > >   static int netboot_common(enum proto_t, struct cmd_tbl *, int, char * const []);
> > >
> > > @@ -524,3 +525,24 @@ U_BOOT_CMD(
> > >       "list - list available devices\n"
> > >   );
> > >   #endif // CONFIG_DM_ETH
> > > +
> > > +#if defined(CONFIG_CMD_NCSI)
> > > +static int do_ncsi(struct cmd_tbl *cmdtp, int flag, int argc, char * const argv[])
> > > +{
> > > +     if (!phy_interface_is_ncsi() || !ncsi_active()) {
> > > +             printf("Device not configured for NC-SI\n");
> > > +             return CMD_RET_FAILURE;
> > > +     }
> > > +
> > > +     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/drivers/net/phy/ncsi.c b/drivers/net/phy/ncsi.c
> > > index bf1e832be9f1..bb7ecebed382 100644
> > > --- a/drivers/net/phy/ncsi.c
> > > +++ b/drivers/net/phy/ncsi.c
> > > @@ -9,6 +9,7 @@
> > >   #include <log.h>
> > >   #include <malloc.h>
> > >   #include <phy.h>
> > > +#include <net.h>
> > >   #include <net/ncsi.h>
> > >   #include <net/ncsi-pkt.h>
> > >   #include <asm/unaligned.h>
> > > diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> > > index 1121b99abff5..d04538838852 100644
> > > --- a/drivers/net/phy/phy.c
> > > +++ b/drivers/net/phy/phy.c
> > > @@ -1026,7 +1026,7 @@ struct phy_device *phy_connect(struct mii_dev *bus, int addr,
> > >   #endif
> > >
> > >   #ifdef CONFIG_PHY_NCSI
> > > -     if (!phydev)
> > > +     if (!phydev && interface == PHY_INTERFACE_MODE_NCSI)
> > >               phydev = phy_device_create(bus, 0, PHY_NCSI_ID, false);
> > >   #endif
> > >
> > > @@ -1101,3 +1101,10 @@ int phy_modify(struct phy_device *phydev, int devad, int regnum, u16 mask,
> > >
> > >       return phy_write(phydev, devad, regnum, (ret & ~mask) | set);
> > >   }
> > > +
> > > +bool phy_interface_is_ncsi(void)
> > > +{
> > > +     struct eth_pdata *pdata = dev_get_plat(eth_get_dev());
> > > +
> > > +     return pdata->phy_interface == PHY_INTERFACE_MODE_NCSI;
> > > +}
> > > diff --git a/net/net.c b/net/net.c
> > > index 81905f631592..a4e645ac4425 100644
> > > --- a/net/net.c
> > > +++ b/net/net.c
> > > @@ -93,6 +93,7 @@
> > >   #include <net.h>
> > >   #include <net/fastboot.h>
> > >   #include <net/tftp.h>
> > > +#include <net/ncsi.h>
> > >   #if defined(CONFIG_CMD_PCAP)
> > >   #include <net/pcap.h>
> > >   #endif
> > > @@ -410,6 +411,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 (phy_interface_is_ncsi() && 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()) {
> > > @@ -423,6 +434,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_PHY_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", image_load_addr);
> > >                       }
> > > -                     if (protocol != NETCONS)
> > > +                     if (protocol != NETCONS && protocol != NCSI)
> > >                               eth_halt();
> > >                       else
> > >                               eth_halt_state_only();
> > > @@ -1321,6 +1338,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
> > >       }
> > >   }
> > > @@ -1381,6 +1403,9 @@ common:
> > >
> > >   #ifdef CONFIG_CMD_RARP
> > >       case RARP:
> > > +#endif
> > > +#ifdef CONFIG_PHY_NCSI
> > > +     case NCSI:
> > >   #endif
> > >       case BOOTP:
> > >       case CDP:
> > > diff --git a/cmd/Kconfig b/cmd/Kconfig
> > > index ad36ae63c659..bbd00707727d 100644
> > > --- a/cmd/Kconfig
> > > +++ b/cmd/Kconfig
> > > @@ -1796,6 +1796,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
> >
> Please separate the command from the ncsi generic code.
> The command interface should be an option that depends on ncsi support.

I don't follow. Do you want them to be separate patches?

Previous versions of the patch meant ncsi required the command to be
enabled, but this is no longer the case.

Re: [PATCH v3 1/3] net: NC-SI setup and handling

[Ramon Fried]

On Mon, Aug 8, 2022 at 9:18 AM Joel Stanley <joel@jms.id.au> wrote:
>
> On Sat, 6 Aug 2022 at 23:54, Ramon Fried <rfried.dev@gmail.com> wrote:
> >
> > On Mon, Jul 4, 2022 at 11:20 AM Cédric Le Goater <clg@kaod.org> wrote:
> > >
> > > On 7/4/22 09:58, Joel Stanley wrote:
> > > > From: Samuel Mendoza-Jonas <sam@mendozajonas.com>
> > > >
> > > > 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>
> > > > Signed-off-by: Joel Stanley <joel@jms.id.au>
> > >
> > >
> > > Reviewed-by: Cédric Le Goater <clg@kaod.org>
> > >
> > > Thanks,
> > >
> > > C.
> > >
> > > > ---
> > > > v3:
> > > >   - Fix compilation. There were no configs that enabled the NCSI phy code
> > > >     so it had bitrotted
> > > >   - Use NCSI_PHY instead of CMD_NCSI so NCSI can work without the command
> > > >   - Add phy_interface_is_ncsi() helper, thanks Cédric for this suggestion
> > > >   - Only create NCSI phy device when driver is configured for it
> > > >
> > > >   include/net.h          |  2 +-
> > > >   include/phy.h          |  2 ++
> > > >   cmd/net.c              | 22 ++++++++++++++++++++++
> > > >   drivers/net/phy/ncsi.c |  1 +
> > > >   drivers/net/phy/phy.c  |  9 ++++++++-
> > > >   net/net.c              | 27 ++++++++++++++++++++++++++-
> > > >   cmd/Kconfig            |  8 ++++++++
> > > >   7 files changed, 68 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/include/net.h b/include/net.h
> > > > index e3889a0bc85e..0681b8246323 100644
> > > > --- a/include/net.h
> > > > +++ b/include/net.h
> > > > @@ -558,7 +558,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, UDP
> > > > +     TFTPSRV, TFTPPUT, LINKLOCAL, FASTBOOT, WOL, UDP, NCSI
> > > >   };
> > > >
> > > >   extern char net_boot_file_name[1024];/* Boot File name */
> > > > diff --git a/include/phy.h b/include/phy.h
> > > > index b32959571069..1e0f8856f629 100644
> > > > --- a/include/phy.h
> > > > +++ b/include/phy.h
> > > > @@ -583,6 +583,8 @@ static inline bool phy_interface_is_sgmii(struct phy_device *phydev)
> > > >               phydev->interface <= PHY_INTERFACE_MODE_QSGMII;
> > > >   }
> > > >
> > > > +bool phy_interface_is_ncsi(void);
> > > > +
> > > >   /* PHY UIDs for various PHYs that are referenced in external code */
> > > >   #define PHY_UID_CS4340              0x13e51002
> > > >   #define PHY_UID_CS4223              0x03e57003
> > > > diff --git a/cmd/net.c b/cmd/net.c
> > > > index 3619c843d838..2863fe768118 100644
> > > > --- a/cmd/net.c
> > > > +++ b/cmd/net.c
> > > > @@ -16,6 +16,7 @@
> > > >   #include <net.h>
> > > >   #include <net/udp.h>
> > > >   #include <net/sntp.h>
> > > > +#include <net/ncsi.h>
> > > >
> > > >   static int netboot_common(enum proto_t, struct cmd_tbl *, int, char * const []);
> > > >
> > > > @@ -524,3 +525,24 @@ U_BOOT_CMD(
> > > >       "list - list available devices\n"
> > > >   );
> > > >   #endif // CONFIG_DM_ETH
> > > > +
> > > > +#if defined(CONFIG_CMD_NCSI)
> > > > +static int do_ncsi(struct cmd_tbl *cmdtp, int flag, int argc, char * const argv[])
> > > > +{
> > > > +     if (!phy_interface_is_ncsi() || !ncsi_active()) {
> > > > +             printf("Device not configured for NC-SI\n");
> > > > +             return CMD_RET_FAILURE;
> > > > +     }
> > > > +
> > > > +     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/drivers/net/phy/ncsi.c b/drivers/net/phy/ncsi.c
> > > > index bf1e832be9f1..bb7ecebed382 100644
> > > > --- a/drivers/net/phy/ncsi.c
> > > > +++ b/drivers/net/phy/ncsi.c
> > > > @@ -9,6 +9,7 @@
> > > >   #include <log.h>
> > > >   #include <malloc.h>
> > > >   #include <phy.h>
> > > > +#include <net.h>
> > > >   #include <net/ncsi.h>
> > > >   #include <net/ncsi-pkt.h>
> > > >   #include <asm/unaligned.h>
> > > > diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> > > > index 1121b99abff5..d04538838852 100644
> > > > --- a/drivers/net/phy/phy.c
> > > > +++ b/drivers/net/phy/phy.c
> > > > @@ -1026,7 +1026,7 @@ struct phy_device *phy_connect(struct mii_dev *bus, int addr,
> > > >   #endif
> > > >
> > > >   #ifdef CONFIG_PHY_NCSI
> > > > -     if (!phydev)
> > > > +     if (!phydev && interface == PHY_INTERFACE_MODE_NCSI)
> > > >               phydev = phy_device_create(bus, 0, PHY_NCSI_ID, false);
> > > >   #endif
> > > >
> > > > @@ -1101,3 +1101,10 @@ int phy_modify(struct phy_device *phydev, int devad, int regnum, u16 mask,
> > > >
> > > >       return phy_write(phydev, devad, regnum, (ret & ~mask) | set);
> > > >   }
> > > > +
> > > > +bool phy_interface_is_ncsi(void)
> > > > +{
> > > > +     struct eth_pdata *pdata = dev_get_plat(eth_get_dev());
> > > > +
> > > > +     return pdata->phy_interface == PHY_INTERFACE_MODE_NCSI;
> > > > +}
> > > > diff --git a/net/net.c b/net/net.c
> > > > index 81905f631592..a4e645ac4425 100644
> > > > --- a/net/net.c
> > > > +++ b/net/net.c
> > > > @@ -93,6 +93,7 @@
> > > >   #include <net.h>
> > > >   #include <net/fastboot.h>
> > > >   #include <net/tftp.h>
> > > > +#include <net/ncsi.h>
> > > >   #if defined(CONFIG_CMD_PCAP)
> > > >   #include <net/pcap.h>
> > > >   #endif
> > > > @@ -410,6 +411,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 (phy_interface_is_ncsi() && 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()) {
> > > > @@ -423,6 +434,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_PHY_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", image_load_addr);
> > > >                       }
> > > > -                     if (protocol != NETCONS)
> > > > +                     if (protocol != NETCONS && protocol != NCSI)
> > > >                               eth_halt();
> > > >                       else
> > > >                               eth_halt_state_only();
> > > > @@ -1321,6 +1338,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
> > > >       }
> > > >   }
> > > > @@ -1381,6 +1403,9 @@ common:
> > > >
> > > >   #ifdef CONFIG_CMD_RARP
> > > >       case RARP:
> > > > +#endif
> > > > +#ifdef CONFIG_PHY_NCSI
> > > > +     case NCSI:
> > > >   #endif
> > > >       case BOOTP:
> > > >       case CDP:
> > > > diff --git a/cmd/Kconfig b/cmd/Kconfig
> > > > index ad36ae63c659..bbd00707727d 100644
> > > > --- a/cmd/Kconfig
> > > > +++ b/cmd/Kconfig
> > > > @@ -1796,6 +1796,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
> > >
> > Please separate the command from the ncsi generic code.
> > The command interface should be an option that depends on ncsi support.
>
> I don't follow. Do you want them to be separate patches?
Yes. one for the implementation and one for command.