<meta http-equiv="Content-Type" content="text/html; charset=utf-8"><div dir="ltr"><div>The intent of this patch is fine, but changes should be purely additive and properly wrapped in the __rtems__ check. The addition of thread startup at the end should also be wrapped in !NO_SYS.</div><div><br></div><div>Kinsey<br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sun, Mar 31, 2024 at 5:49 PM Bernd Moessner <<a href="mailto:berndmoessner80@gmail.com">berndmoessner80@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Within xemac_add, the link_detect_thread is set up before the ethernet<br>
interface is configured and all data structures have been set up correctly.<br>
The steps within xemac_add are basically:<br>
1) Set up link_detect_thread<br>
2) Initialize the interface<br>
3) Set up the link speed / start autonegotiation<br>
4) When autonegotiate has completed, set up the data structures<br>
<br>
The issue is here that, for example if no cable is connected, the<br>
autonegotiate sequence will suspend the calling task. As soon as<br>
this happens, the link_detect_thread gets into running state and<br>
dereferences the data structures that havent been set up. Unfortunatelly,<br>
it isnt as easy as checking the pointer to the relevant data structure<br>
for being NULL within the link_detect_thread. Not going into details,<br>
the pointer (netif->state) isnt NULL in this moment of time, but it<br>
has to point to something completely different.<br>
<br>
It seems to me that the easiest solution to this problem is to reorder<br>
the sequence within xemac_add. Initialize the interface and set up the<br>
data structures, then create the link_detect_thread which requires all of<br>
this. After applying this patch, the sequence becomes:<br>
<br>
1) Initialize the interface<br>
2) Set up the link speed / start autonegotiation<br>
3) When autonegotiate has completed, set up the data structures<br>
4) Set up link_detect_thread<br>
---<br>
.../src/contrib/ports/xilinx/netif/xadapter.c | 32 ++++++++++++++-----<br>
1 file changed, 24 insertions(+), 8 deletions(-)<br>
<br>
diff --git a/embeddedsw/ThirdParty/sw_services/lwip211/src/contrib/ports/xilinx/netif/xadapter.c b/embeddedsw/ThirdParty/sw_services/lwip211/src/contrib/ports/xilinx/netif/xadapter.c<br>
index 93ff148..79a10c1 100644<br>
--- a/embeddedsw/ThirdParty/sw_services/lwip211/src/contrib/ports/xilinx/netif/xadapter.c<br>
+++ b/embeddedsw/ThirdParty/sw_services/lwip211/src/contrib/ports/xilinx/netif/xadapter.c<br>
@@ -126,8 +126,9 @@ xemac_add(struct netif *netif,<br>
UINTPTR mac_baseaddr)<br>
{<br>
int i;<br>
+ struct netif * ret = NULL;<br>
<br>
-#if !NO_SYS<br>
+#if !NO_SYS && !defined(__rtems__)<br>
/* Start thread to detect link periodically for Hot Plug autodetect */<br>
sys_thread_new("link_detect_thread", link_detect_thread, netif,<br>
THREAD_STACKSIZE, tskIDLE_PRIORITY);<br>
@@ -142,7 +143,7 @@ xemac_add(struct netif *netif,<br>
switch (find_mac_type(mac_baseaddr)) {<br>
case xemac_type_xps_emaclite:<br>
#ifdef XLWIP_CONFIG_INCLUDE_EMACLITE<br>
- return netif_add(netif, ipaddr, netmask, gw,<br>
+ ret = netif_add(netif, ipaddr, netmask, gw,<br>
(void*)mac_baseaddr,<br>
xemacliteif_init,<br>
#if NO_SYS<br>
@@ -151,12 +152,14 @@ xemac_add(struct netif *netif,<br>
tcpip_input<br>
#endif<br>
);<br>
+ break;<br>
#else<br>
- return NULL;<br>
+ ret = NULL;<br>
+ break;<br>
#endif<br>
case xemac_type_axi_ethernet:<br>
#ifdef XLWIP_CONFIG_INCLUDE_AXI_ETHERNET<br>
- return netif_add(netif, ipaddr, netmask, gw,<br>
+ ret = netif_add(netif, ipaddr, netmask, gw,<br>
(void*)mac_baseaddr,<br>
xaxiemacif_init,<br>
#if NO_SYS<br>
@@ -165,16 +168,18 @@ xemac_add(struct netif *netif,<br>
tcpip_input<br>
#endif<br>
);<br>
+ break;<br>
#else<br>
- return NULL;<br>
+ ret = NULL;<br>
+ break;<br>
#endif<br>
#if defined (__arm__) || defined (__aarch64__)<br>
case xemac_type_emacps:<br>
#ifdef XLWIP_CONFIG_INCLUDE_GEM<br>
#ifndef __rtems__<br>
- return netif_add(netif, ipaddr, netmask, gw,<br>
+ ret = netif_add(netif, ipaddr, netmask, gw,<br>
#else /* __rtems__ */<br>
- return netif_add( netif,<br>
+ ret = netif_add( netif,<br>
(const ip4_addr_t *) ipaddr,<br>
(const ip4_addr_t *) netmask,<br>
(const ip4_addr_t *) gw,<br>
@@ -188,7 +193,10 @@ xemac_add(struct netif *netif,<br>
#endif<br>
<br>
);<br>
+ break;<br>
#endif<br>
+ ret = NULL;<br>
+ break;<br>
#endif<br>
default:<br>
#ifndef __rtems__<br>
@@ -199,8 +207,16 @@ xemac_add(struct netif *netif,<br>
mac_baseaddr);<br>
xil_printf("\r\n");<br>
#endif<br>
- return NULL;<br>
+ ret = NULL;<br>
}<br>
+<br>
+#if defined(__rtems__)<br>
+ /* Start thread to detect link periodically for Hot Plug autodetect */<br>
+ sys_thread_new("link_detect_thread", link_detect_thread, netif,<br>
+ THREAD_STACKSIZE, tskIDLE_PRIORITY);<br>
+#endif<br>
+<br>
+ return ret;<br>
}<br>
<br>
#if !NO_SYS<br>
-- <br>
2.34.1<br>
<br>
_______________________________________________<br>
devel mailing list<br>
<a href="mailto:devel@rtems.org" target="_blank">devel@rtems.org</a><br>
<a href="http://lists.rtems.org/mailman/listinfo/devel" rel="noreferrer" target="_blank">http://lists.rtems.org/mailman/listinfo/devel</a><br>
</blockquote></div>