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.
Kinsey
On Sun, Mar 31, 2024 at 5:49 PM Bernd Moessner <berndmoessne...@gmail.com>
wrote:
> Within xemac_add, the link_detect_thread is set up before the ethernet
> interface is configured and all data structures have been set up correctly.
> The steps within xemac_add are basically:
> 1) Set up link_detect_thread
> 2) Initialize the interface
> 3) Set up the link speed / start autonegotiation
> 4) When autonegotiate has completed, set up the data structures
>
> The issue is here that, for example if no cable is connected, the
> autonegotiate sequence will suspend the calling task. As soon as
> this happens, the link_detect_thread gets into running state and
> dereferences the data structures that havent been set up. Unfortunatelly,
> it isnt as easy as checking the pointer to the relevant data structure
> for being NULL within the link_detect_thread. Not going into details,
> the pointer (netif->state) isnt NULL in this moment of time, but it
> has to point to something completely different.
>
> It seems to me that the easiest solution to this problem is to reorder
> the sequence within xemac_add. Initialize the interface and set up the
> data structures, then create the link_detect_thread which requires all of
> this. After applying this patch, the sequence becomes:
>
> 1) Initialize the interface
> 2) Set up the link speed / start autonegotiation
> 3) When autonegotiate has completed, set up the data structures
> 4) Set up link_detect_thread
> ---
> .../src/contrib/ports/xilinx/netif/xadapter.c | 32 ++++++++++++++-----
> 1 file changed, 24 insertions(+), 8 deletions(-)
>
> 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
> index 93ff148..79a10c1 100644
> ---
> 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
> @@ -126,8 +126,9 @@ xemac_add(struct netif *netif,
> UINTPTR mac_baseaddr)
> {
> int i;
> + struct netif * ret = NULL;
>
> -#if !NO_SYS
> +#if !NO_SYS && !defined(__rtems__)
> /* Start thread to detect link periodically for Hot Plug
> autodetect */
> sys_thread_new("link_detect_thread", link_detect_thread, netif,
> THREAD_STACKSIZE, tskIDLE_PRIORITY);
> @@ -142,7 +143,7 @@ xemac_add(struct netif *netif,
> switch (find_mac_type(mac_baseaddr)) {
> case xemac_type_xps_emaclite:
> #ifdef XLWIP_CONFIG_INCLUDE_EMACLITE
> - return netif_add(netif, ipaddr, netmask,
> gw,
> + ret = netif_add(netif, ipaddr, netmask, gw,
> (void*)mac_baseaddr,
> xemacliteif_init,
> #if NO_SYS
> @@ -151,12 +152,14 @@ xemac_add(struct netif *netif,
> tcpip_input
> #endif
> );
> + break;
> #else
> - return NULL;
> + ret = NULL;
> + break;
> #endif
> case xemac_type_axi_ethernet:
> #ifdef XLWIP_CONFIG_INCLUDE_AXI_ETHERNET
> - return netif_add(netif, ipaddr, netmask,
> gw,
> + ret = netif_add(netif, ipaddr, netmask, gw,
> (void*)mac_baseaddr,
> xaxiemacif_init,
> #if NO_SYS
> @@ -165,16 +168,18 @@ xemac_add(struct netif *netif,
> tcpip_input
> #endif
> );
> + break;
> #else
> - return NULL;
> + ret = NULL;
> + break;
> #endif
> #if defined (__arm__) || defined (__aarch64__)
> case xemac_type_emacps:
> #ifdef XLWIP_CONFIG_INCLUDE_GEM
> #ifndef __rtems__
> - return netif_add(netif, ipaddr, netmask,
> gw,
> + ret = netif_add(netif, ipaddr, netmask, gw,
> #else /* __rtems__ */
> - return netif_add( netif,
> + ret = netif_add( netif,
> (const ip4_addr_t *)
> ipaddr,
> (const ip4_addr_t *)
> netmask,
> (const ip4_addr_t *) gw,
> @@ -188,7 +193,10 @@ xemac_add(struct netif *netif,
> #endif
>
> );
> + break;
> #endif
> + ret = NULL;
> + break;
> #endif
> default:
> #ifndef __rtems__
> @@ -199,8 +207,16 @@ xemac_add(struct netif *netif,
> mac_baseaddr);
> xil_printf("\r\n");
> #endif
> - return NULL;
> + ret = NULL;
> }
> +
> +#if defined(__rtems__)
> + /* Start thread to detect link periodically for Hot Plug
> autodetect */
> + sys_thread_new("link_detect_thread", link_detect_thread, netif,
> + THREAD_STACKSIZE, tskIDLE_PRIORITY);
> +#endif
> +
> + return ret;
> }
>
> #if !NO_SYS
> --
> 2.34.1
>
> _______________________________________________
> devel mailing list
> devel@rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
>
_______________________________________________
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
