[U-Boot] [PATCH v2 0/5] fix NetConsole for CONFIG_DM_ETH
With the introduction of driver model and accompanying changes, outdated code in netconsole.c leads to compilation errors when both CONFIG_NETCONSOLE and CONFIG_DM_ETH are set.
This is a series of patches to fix these issues and get NetConsole working again with DM_ETH. v2 adds some proper subsystem prefixes on the commit messages, and enables a test case (sunxi board configuration for Banana Pi/Pro) that combines CONFIG_DM_ETH and CONFIG_NETCONSOLE.
I have tested the resulting code on my Banana Pi (sun7i / Allwinner A20) and had a functional NetConsole again. I also backported eth_is_active() and the netconsole.c changes to v2015.04, to make sure they properly worked in case CONFIG_DM_ETH is absent (v2015.04 predates the introduction of DM_ETH, and I'm lacking other hardware to test CONFIG_NETCONSOLE with).
Regards, B. Nortmann
Changes in v2: - add "net:" prefix to commit message - add "net:" prefix to commit message - add "net:" prefix to commit message
Bernhard Nortmann (5): net: expose eth_is_active() function to test network device state net: fix netconsole when CONFIG_DM_ETH is set net: avoid eth_unregister() call when function is unavailable net: support NETCONSOLE option via Kconfig sunxi: add NetConsole by default for Banana Pi/Pro
common/bootm.c | 2 ++ configs/Bananapi_defconfig | 3 ++- configs/Bananapro_defconfig | 3 ++- drivers/net/netconsole.c | 14 +++++++++++--- include/net.h | 6 ++++++ net/Kconfig | 6 ++++++ net/eth.c | 18 +++++++++++++++++- 7 files changed, 46 insertions(+), 6 deletions(-)
The previous eth_device struct returned by eth_get_dev() allowed code to directly query the state member field. However, with CONFIG_DM_ETH this data gets encapsulated (i.e. private), and eth_get_dev() returns a udevice struct 'abstraction' instead.
This breaks legacy code relying on the former behaviour - e.g. netconsole. (see http://lists.denx.de/pipermail/u-boot/2015-June/216528.html)
The patch introduces a method to retrieve the ethernet device state in a 'clean' and uniform way, supporting both legacy code and driver model. The new function eth_is_active() accepts a device struct pointer and tests it for ETH_STATE_ACTIVE.
Signed-off-by: Bernhard Nortmann bernhard.nortmann@web.de Reviewed-by: Simon Glass sjg@chromium.org ---
Changes in v2: - add "net:" prefix to commit message
include/net.h | 6 ++++++ net/eth.c | 18 +++++++++++++++++- 2 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/include/net.h b/include/net.h index d09bec9..c135ec4 100644 --- a/include/net.h +++ b/include/net.h @@ -149,7 +149,13 @@ struct udevice *eth_get_dev(void); /* get the current device */ */ struct udevice *eth_get_dev_by_name(const char *devname); unsigned char *eth_get_ethaddr(void); /* get the current device MAC */ + /* Used only when NetConsole is enabled */ +# ifdef CONFIG_DM_ETH +int eth_is_active(struct udevice *dev); /* Test device for active state */ +# else +int eth_is_active(struct eth_device *dev); /* Test device for active state */ +# endif int eth_init_state_only(void); /* Set active state */ void eth_halt_state_only(void); /* Set passive state */ #endif diff --git a/net/eth.c b/net/eth.c index d3ec8d6..d3ff7c6 100644 --- a/net/eth.c +++ b/net/eth.c @@ -386,6 +386,17 @@ void eth_halt(void) priv->state = ETH_STATE_PASSIVE; }
+int eth_is_active(struct udevice *dev) +{ + struct eth_device_priv *priv; + + if (!dev || !device_active(dev)) + return 0; + + priv = dev->uclass_priv; + return priv->state == ETH_STATE_ACTIVE; +} + int eth_send(void *packet, int length) { struct udevice *current; @@ -577,7 +588,7 @@ UCLASS_DRIVER(eth) = { .per_device_auto_alloc_size = sizeof(struct eth_device_priv), .flags = DM_UC_FLAG_SEQ_ALIAS, }; -#endif +#endif /* #ifdef CONFIG_DM_ETH */
#ifndef CONFIG_DM_ETH
@@ -915,6 +926,11 @@ void eth_halt(void) eth_current->state = ETH_STATE_PASSIVE; }
+int eth_is_active(struct eth_device *dev) +{ + return dev && dev->state == ETH_STATE_ACTIVE; +} + int eth_send(void *packet, int length) { if (!eth_current)
Hi Bernhard,
On Wed, Aug 26, 2015 at 7:35 AM, Bernhard Nortmann bernhard.nortmann@web.de wrote:
The previous eth_device struct returned by eth_get_dev() allowed code to directly query the state member field. However, with CONFIG_DM_ETH this data gets encapsulated (i.e. private), and eth_get_dev() returns a udevice struct 'abstraction' instead.
This breaks legacy code relying on the former behaviour - e.g. netconsole. (see http://lists.denx.de/pipermail/u-boot/2015-June/216528.html)
The patch introduces a method to retrieve the ethernet device state in a 'clean' and uniform way, supporting both legacy code and driver model. The new function eth_is_active() accepts a device struct pointer and tests it for ETH_STATE_ACTIVE.
Signed-off-by: Bernhard Nortmann bernhard.nortmann@web.de Reviewed-by: Simon Glass sjg@chromium.org
Changes in v2:
- add "net:" prefix to commit message
include/net.h | 6 ++++++ net/eth.c | 18 +++++++++++++++++- 2 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/include/net.h b/include/net.h index d09bec9..c135ec4 100644 --- a/include/net.h +++ b/include/net.h @@ -149,7 +149,13 @@ struct udevice *eth_get_dev(void); /* get the current device */ */ struct udevice *eth_get_dev_by_name(const char *devname); unsigned char *eth_get_ethaddr(void); /* get the current device MAC */
/* Used only when NetConsole is enabled */ +# ifdef CONFIG_DM_ETH +int eth_is_active(struct udevice *dev); /* Test device for active state */ +# else +int eth_is_active(struct eth_device *dev); /* Test device for active state */ +# endif int eth_init_state_only(void); /* Set active state */ void eth_halt_state_only(void); /* Set passive state */ #endif diff --git a/net/eth.c b/net/eth.c index d3ec8d6..d3ff7c6 100644 --- a/net/eth.c +++ b/net/eth.c @@ -386,6 +386,17 @@ void eth_halt(void) priv->state = ETH_STATE_PASSIVE; }
+int eth_is_active(struct udevice *dev) +{
struct eth_device_priv *priv;if (!dev || !device_active(dev))return 0;priv = dev->uclass_priv;
You should use dev_get_uclass_priv();
return priv->state == ETH_STATE_ACTIVE;+}
int eth_send(void *packet, int length) { struct udevice *current; @@ -577,7 +588,7 @@ UCLASS_DRIVER(eth) = { .per_device_auto_alloc_size = sizeof(struct eth_device_priv), .flags = DM_UC_FLAG_SEQ_ALIAS, }; -#endif +#endif /* #ifdef CONFIG_DM_ETH */
#ifndef CONFIG_DM_ETH
@@ -915,6 +926,11 @@ void eth_halt(void) eth_current->state = ETH_STATE_PASSIVE; }
+int eth_is_active(struct eth_device *dev) +{
return dev && dev->state == ETH_STATE_ACTIVE;+}
int eth_send(void *packet, int length) { if (!eth_current) -- 2.4.6
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
The previous eth_device struct returned by eth_get_dev() allowed code to directly query the state member field. However, with CONFIG_DM_ETH this data gets encapsulated (i.e. private), and eth_get_dev() returns a udevice struct 'abstraction' instead.
This breaks legacy code relying on the former behaviour - e.g. netconsole. (see http://lists.denx.de/pipermail/u-boot/2015-June/216528.html)
The patch introduces a method to retrieve the ethernet device state in a 'clean' and uniform way, supporting both legacy code and driver model. The new function eth_is_active() accepts a device struct pointer and tests it for ETH_STATE_ACTIVE.
Series-changes: 3 - use dev_get_uclass_priv()
Signed-off-by: Bernhard Nortmann bernhard.nortmann@web.de Reviewed-by: Simon Glass sjg@chromium.org --- include/net.h | 6 ++++++ net/eth.c | 18 +++++++++++++++++- 2 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/include/net.h b/include/net.h index d09bec9..c135ec4 100644 --- a/include/net.h +++ b/include/net.h @@ -149,7 +149,13 @@ struct udevice *eth_get_dev(void); /* get the current device */ */ struct udevice *eth_get_dev_by_name(const char *devname); unsigned char *eth_get_ethaddr(void); /* get the current device MAC */ + /* Used only when NetConsole is enabled */ +# ifdef CONFIG_DM_ETH +int eth_is_active(struct udevice *dev); /* Test device for active state */ +# else +int eth_is_active(struct eth_device *dev); /* Test device for active state */ +# endif int eth_init_state_only(void); /* Set active state */ void eth_halt_state_only(void); /* Set passive state */ #endif diff --git a/net/eth.c b/net/eth.c index d3ec8d6..39b9918 100644 --- a/net/eth.c +++ b/net/eth.c @@ -386,6 +386,17 @@ void eth_halt(void) priv->state = ETH_STATE_PASSIVE; }
+int eth_is_active(struct udevice *dev) +{ + struct eth_device_priv *priv; + + if (!dev || !device_active(dev)) + return 0; + + priv = dev_get_uclass_priv(dev); + return priv->state == ETH_STATE_ACTIVE; +} + int eth_send(void *packet, int length) { struct udevice *current; @@ -577,7 +588,7 @@ UCLASS_DRIVER(eth) = { .per_device_auto_alloc_size = sizeof(struct eth_device_priv), .flags = DM_UC_FLAG_SEQ_ALIAS, }; -#endif +#endif /* #ifdef CONFIG_DM_ETH */
#ifndef CONFIG_DM_ETH
@@ -915,6 +926,11 @@ void eth_halt(void) eth_current->state = ETH_STATE_PASSIVE; }
+int eth_is_active(struct eth_device *dev) +{ + return dev && dev->state == ETH_STATE_ACTIVE; +} + int eth_send(void *packet, int length) { if (!eth_current)
This patches uses the eth_is_active() function to work around issues that prevented compilation with the newer driver model.
Signed-off-by: Bernhard Nortmann bernhard.nortmann@web.de ---
Changes in v2: - add "net:" prefix to commit message
drivers/net/netconsole.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c index 31042a6..bf972dc 100644 --- a/drivers/net/netconsole.c +++ b/drivers/net/netconsole.c @@ -170,7 +170,11 @@ int nc_input_packet(uchar *pkt, struct in_addr src_ip, unsigned dest_port,
static void nc_send_packet(const char *buf, int len) { +#ifdef CONFIG_DM_ETH + struct udevice *eth; +#else struct eth_device *eth; +#endif int inited = 0; uchar *pkt; uchar *ether; @@ -183,7 +187,7 @@ static void nc_send_packet(const char *buf, int len) return;
if (!memcmp(nc_ether, net_null_ethaddr, 6)) { - if (eth->state == ETH_STATE_ACTIVE) + if (eth_is_active(eth)) return; /* inside net loop */ output_packet = buf; output_packet_len = len; @@ -194,7 +198,7 @@ static void nc_send_packet(const char *buf, int len) return; }
- if (eth->state != ETH_STATE_ACTIVE) { + if (!eth_is_active(eth)) { if (eth_is_on_demand_init()) { if (eth_init() < 0) return; @@ -292,7 +296,11 @@ static int nc_stdio_getc(struct stdio_dev *dev)
static int nc_stdio_tstc(struct stdio_dev *dev) { +#ifdef CONFIG_DM_ETH + struct udevice *eth; +#else struct eth_device *eth; +#endif
if (input_recursion) return 0; @@ -301,7 +309,7 @@ static int nc_stdio_tstc(struct stdio_dev *dev) return 1;
eth = eth_get_dev(); - if (eth && eth->state == ETH_STATE_ACTIVE) + if (eth_is_active(eth)) return 0; /* inside net loop */
input_recursion = 1;
On Wed, Aug 26, 2015 at 7:35 AM, Bernhard Nortmann bernhard.nortmann@web.de wrote:
This patches uses the eth_is_active() function to work around issues that prevented compilation with the newer driver model.
Signed-off-by: Bernhard Nortmann bernhard.nortmann@web.de
Acked-by: Joe Hershberger joe.hershberger@ni.com
CONFIG_NETCONSOLE causes common/bootm.c to call eth_unregister() for network device shutdown. However, with CONFIG_DM_ETH this function is no longer defined.
This is a workaround to avoid the call in that case, and solely rely on eth_halt(). In case this is insufficient, a proper way to unregister / remove network devices needs to be implemented.
Signed-off-by: Bernhard Nortmann bernhard.nortmann@web.de Reviewed-by: Simon Glass sjg@chromium.org ---
Changes in v2: - add "net:" prefix to commit message
common/bootm.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/common/bootm.c b/common/bootm.c index 667c934..c0d0d09 100644 --- a/common/bootm.c +++ b/common/bootm.c @@ -474,7 +474,9 @@ ulong bootm_disable_interrupts(void) #ifdef CONFIG_NETCONSOLE /* Stop the ethernet stack if NetConsole could have left it up */ eth_halt(); +# ifndef CONFIG_DM_ETH eth_unregister(eth_get_dev()); +# endif #endif
#if defined(CONFIG_CMD_USB)
On Wed, Aug 26, 2015 at 7:35 AM, Bernhard Nortmann bernhard.nortmann@web.de wrote:
CONFIG_NETCONSOLE causes common/bootm.c to call eth_unregister() for network device shutdown. However, with CONFIG_DM_ETH this function is no longer defined.
This is a workaround to avoid the call in that case, and solely rely on eth_halt(). In case this is insufficient, a proper way to unregister / remove network devices needs to be implemented.
Signed-off-by: Bernhard Nortmann bernhard.nortmann@web.de Reviewed-by: Simon Glass sjg@chromium.org
Acked-by: Joe Hershberger joe.hershberger@ni.com
This patch introduces CONFIG_NETCONSOLE as an option to the Kconfig system.
Joe Hershberger pointed out that it may not be entirely free of problems, as many boards predating the driver model define this symbol directly via include files. In case they're not properly migrated, their NetConsole might 'vanish' if they start to use CONFIG_NET or CONFIG_NETDEVICES.
Signed-off-by: Bernhard Nortmann bernhard.nortmann@web.de ---
Changes in v2: None
net/Kconfig | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/net/Kconfig b/net/Kconfig index 915371d..77a2f7e 100644 --- a/net/Kconfig +++ b/net/Kconfig @@ -16,4 +16,10 @@ config NET_RANDOM_ETHADDR A new MAC address will be generated on every boot and it will not be added to the environment.
+config NETCONSOLE + bool "NetConsole support" + help + Support the 'nc' input/output device for networked console. + See README.NetConsole for details. + endif # if NET
On Wed, Aug 26, 2015 at 7:35 AM, Bernhard Nortmann bernhard.nortmann@web.de wrote:
This patch introduces CONFIG_NETCONSOLE as an option to the Kconfig system.
Joe Hershberger pointed out that it may not be entirely free of problems, as many boards predating the driver model define this symbol directly via include files. In case they're not properly migrated, their NetConsole might 'vanish' if they start to use CONFIG_NET or CONFIG_NETDEVICES.
Signed-off-by: Bernhard Nortmann bernhard.nortmann@web.de
Acked-by: Joe Hershberger joe.hershberger@ni.com
Do you plan to use moveconfig.pl to update other boards?
I guess that can wait until next release.
-Joe
Simon Glass and Joe Hershberger suggested adding at least one test case for the CONFIG_DM_ETH plus CONFIG_NETCONSOLE options.
This patch enables NetConsole as a default for the "Banana Pi/Pro" sunxi boards.
(By the nature of this patch it could probably be extended later to include all sunxi boards using CONFIG_SUNXI_[EG]MAC.)
Signed-off-by: Bernhard Nortmann bernhard.nortmann@web.de
---
Changes in v2: None
configs/Bananapi_defconfig | 3 ++- configs/Bananapro_defconfig | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/configs/Bananapi_defconfig b/configs/Bananapi_defconfig index 560295f..898631d 100644 --- a/configs/Bananapi_defconfig +++ b/configs/Bananapi_defconfig @@ -2,8 +2,8 @@ CONFIG_ARM=y CONFIG_ARCH_SUNXI=y CONFIG_MACH_SUN7I=y CONFIG_DRAM_CLK=432 -CONFIG_GMAC_TX_DELAY=3 CONFIG_VIDEO_COMPOSITE=y +CONFIG_GMAC_TX_DELAY=3 CONFIG_DEFAULT_DEVICE_TREE="sun7i-a20-bananapi" # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set CONFIG_SPL=y @@ -11,5 +11,6 @@ CONFIG_SYS_EXTRA_OPTIONS="AXP209_POWER,SUNXI_GMAC,RGMII,MACPWR=SUNXI_GPH(23),AHC # CONFIG_CMD_IMLS is not set # CONFIG_CMD_FLASH is not set # CONFIG_CMD_FPGA is not set +CONFIG_NETCONSOLE=y CONFIG_ETH_DESIGNWARE=y CONFIG_USB_EHCI_HCD=y diff --git a/configs/Bananapro_defconfig b/configs/Bananapro_defconfig index 346db34..e9909d9 100644 --- a/configs/Bananapro_defconfig +++ b/configs/Bananapro_defconfig @@ -4,8 +4,8 @@ CONFIG_MACH_SUN7I=y CONFIG_DRAM_CLK=432 CONFIG_USB1_VBUS_PIN="PH0" CONFIG_USB2_VBUS_PIN="PH1" -CONFIG_GMAC_TX_DELAY=3 CONFIG_VIDEO_COMPOSITE=y +CONFIG_GMAC_TX_DELAY=3 CONFIG_DEFAULT_DEVICE_TREE="sun7i-a20-bananapro" # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set CONFIG_SPL=y @@ -13,5 +13,6 @@ CONFIG_SYS_EXTRA_OPTIONS="AXP209_POWER,SUNXI_GMAC,RGMII,MACPWR=SUNXI_GPH(23),AHC # CONFIG_CMD_IMLS is not set # CONFIG_CMD_FLASH is not set # CONFIG_CMD_FPGA is not set +CONFIG_NETCONSOLE=y CONFIG_ETH_DESIGNWARE=y CONFIG_USB_EHCI_HCD=y
On Wed, Aug 26, 2015 at 7:36 AM, Bernhard Nortmann bernhard.nortmann@web.de wrote:
Simon Glass and Joe Hershberger suggested adding at least one test case for the CONFIG_DM_ETH plus CONFIG_NETCONSOLE options.
This patch enables NetConsole as a default for the "Banana Pi/Pro" sunxi boards.
(By the nature of this patch it could probably be extended later to include all sunxi boards using CONFIG_SUNXI_[EG]MAC.)
Signed-off-by: Bernhard Nortmann bernhard.nortmann@web.de
Acked-by: Joe Hershberger joe.hershberger@ni.com
participants (2)
-
Bernhard Nortmann -
Joe Hershberger