[bug fixes] STM32H7 Ethernet

[alister]

@Amel NASRI​, @ranran​, @Piranha​, @Harrold​, @Pavel A.​ 

V2 of my fixes and improvements to H7_FW V1.5.0/V1.6.0 Ethernet...

Changes include

• Decoupling receive buffers from receive descriptors so buffers may be held any length time without choking receive.
• Optionally queuing transmit, so transmit doesn't need to block until complete.
• Many bug fixes.

Find full details and source in the attached zip. V1 was posted to another developer's question. Please post any questions about V2 here.

[alister]

Errata to V2.0 documentation:

• “MEMP_NUM_PBUF should be 0�? is incorrect. It’s needed for tcp_write to use PBUF_ROM where data isn’t copied.
• LOCK_TCPIP_CORE/UNLOCK_TCPIP_CORE in ethernetif_input is mentioned twice. The first mention is correct. It’s not necessary and should be removed because tcpip_inpkt manages threading by message queue.
• On page 10 describing integrating the changes with your Cube project, should include: you must copy pertinent changes from my ethernetif.c to yours. Your ethernetif.c will be different because you'll have different phy, you may use different receive buffer sizes and counts, you may configure your MPU differently etc. As is, it won't even compile, as I've removed ethernet_link_thread and maybe other things, which you'll have to write yourself.

[Anand Ram]

hi , can you please do share the user code also eg: #include "eth.h" and its sources.

[Singh.Harjit]

How about putting it on Github so that it can be maintained?

[alister]

> please do share the user code also eg: #include "eth.h" and its sources.

No I'll not share my phy code. This is like the BSP code of one of ST's example projects. You have to write this part yourself. Most likely it is limited value to you anyway. I've provided all the source files that ST's Cube would provide.

[edit] actually I've replaced MX_LWIP_Init and hadn't included that. FW_H7_V1.6.0 moves its call to MX_LWIP_Init to main, which is too early for my app, So in a USER CODE area above main() in main.c I've added “#define MX_LWIP_Init() /* Not used. */�? (without the quotes) and I've excluded LWIP\App\lwip.c from my builds and I've written my own. Other developers have posted Community already about mistakes in MX_LWIP_Init, e.g., incorrectly using netif_* where it should use netifapi_netif_*.

[alister]

> How about putting it on Github so that it can be maintained?

I don't have a development board. So no common platform. Everyone's ethernetif.c would be different.

Finally, the ethernet bug-fixes task is finished where I work and I've other matters to attend.

[alister]

To some questions from https://community.st.com/s/question/0D70X000007QuPr/cant-receive-ethernet-packets-in-stm32h750vb...

> tried to protect 32KB which covers all RX/TX buffers. It did not work.

Not sure. I'll describe the memories a bit. Hopefully something here helps.

Three memories areas need care:

1. Rx buffers. DMA writes direct to memory. If it's cached (a) you'll need to invalidate the buffer using SCB_InvalidateDCache_by_Addr after the receive is finished and before you read the data and (b) because invalidate would destroy unsaved information, e.g. the lwIP pbuf info, it's important the buffer starts at the start of a cache line and ends at the end of a cache line. On the H7 cache lines are 32-bytes.
2. Tx buffers. DMA reads direct from memory. If it's cached you'll have to save the buffer using SCB_CleanDCache_by_Addr before starting transmit. Buffer alignment doesn't matter because no information's lost.
3. DMA descriptors. These need every access to be completed in program order. If you don't use the MPU, even if cache is disabled, you'll run into trouble unless you add memory barriers to the software.

Ideally the rx and tx buffers would be in D2. This is what I've done:

1. Rx buffers: My app needs a lot of rx buffers and D2's used for other stuff, so I've moved rx buffers to D1. Also as memory's scarce, and I know most the packets are small I've reduced the size of my rx buffers. I've used MPU to make my rx buffers not-cached. In testing I didn't see any change to performance between cached and uncached. Understanding what else my app needs to do and that it'd access the data only once I made the call to leave it not-cached.
2. Tx buffers: lwIP allocates tx buffers from its heap. So what you do for tx buffers you're doing for any tables it allocates too. For performance we want the tx buffers in D2 (see AN4891 if this isn't clear). So I've put the entire heap in D2 and made it cached.

For the descriptors, this is what I've done: I'd not added memory barriers. MPU was easier. When ST fix their driver they ought add the necessary memory barriers and make it a Cube configuration so it defines macros that'd compile-in the memory barriers only if MPU's not used because they'd waste cycles if they're not needed.

This snippet from my map file shows the buffers and descriptors. The section names and miss-spellings were there before I arrived.

.EthTxBlock     0x30040000     0x4e30
                0x30040000                . = ALIGN (0x4)
                0x30040000                __ETH_TX_START = .
 *(.TxArraySection)
 .TxArraySection
                0x30040000     0x4e30 Common\Eth\LWIP\Target\ethernetif.o
                0x30040000                lwip_custom_ram_heap
                0x30044e30                __ETH_TX_END = .
 
.EthDescriptorsBlock
                0x30044e30      0x5d0
                0x30045000                . = ALIGN (0x400)
 *fill*         0x30044e30      0x1d0 
                0x30045000                __ETH_DESCRIPTORS_START = .
 *(.RxDecripSection)
 .RxDecripSection
                0x30045000      0x180 Common\Eth\LWIP\Target\ethernetif.o
                0x30045000                DMARxDscrTab
 *(.TxDecripSection)
 .TxDecripSection
                0x30045180      0x180 Common\Eth\LWIP\Target\ethernetif.o
                0x30045180                DMATxDscrTab
                0x30045400                . = ALIGN (0x400)
 *fill*         0x30045300      0x100 
                0x30045400                __ETH_DESCRIPTORS_END = .
 
.EthRxBlock     0x24000000    0x20000
                0x24000000                . = ALIGN (0x20000)
                0x24000000                __ETH_RX_START = .
 *(.RxArraySection)
 .RxArraySection
                0x24000000    0x1fe00 Common\Eth\LWIP\Target\ethernetif.o
                0x24000000                EthIfRxBuff
                0x24020000                . = ALIGN (0x20000)
 *fill*         0x2401fe00      0x200 
                0x24020000                __ETH_RX_END = .

__ETH_TX_START is the start of the lwIP heap. In the source code I've provided, this is cached.

__ETH_DESCRIPTORS_START and section(".RxDecripSection") is the start of the descriptor memory. The section(".TxDecripSection") is a little higher in the memory. In the source code I've provided, this is an MPU region.

__ETH_RX_START and section(".RxArraySection") is the start of my rx buffers. In the source code I've provided, this is not-cached.

> you have used two regions starting from 0x30040000

Check my map-file snippet and details above. Check https://sourceware.org/binutils/docs/ld/SECTIONS.html#SECTIONS.

> where in stm32h7xx_hal_conf.h file I have to add those new defines

You don't. This stm32h7xx_hal_conf.h exists for (a) fixing defines like ETH_TX_DESC_CNT that the current Cube doesn't define properly, and (b) adding defines like ETH_PMT_IT_ENABLED which I've added to ST's ethernet driver source code as an example how they might improve that driver's implementation.

It's ethernetif.c job to integrate lwIP if you use it, your OS if you use one, the ethernet driver and your app.

For my changed ethernetif.c, you need to:

1. Change these macros to suit your needs:
   1. For receive: ETH_RX_BUFFER_SIZE, ETH_RX_BUFFER_COUNT, ETH_RX_BUFFERS_ARE_CACHED
   2. For transmit: ETH_TX_BUFFERS_ARE_CACHED, ETH_TX_QUEUE_ENABLE
2. Place these definitions correctly: DMARxDscrTab, DMATxDscrTab and EthIfRxBuff. You do that in your linker script file or elsewhere in your IDE. I use GCC and haven't fixed or tested anything else. I see __ICCARM__ and __CC_ARM use dodgy addresses. That'd be a problem. If you use them you'll need to change those. I suggest find a better technique and notify ST how because it's better that their future spin of ethernetif.c needs no changes outside of USER CODE blocks.

> I see that HArdFault_Handler() happens immediately

Perhaps D2 needs its clock enabled. See AN4891. In rev 3 it's on page 19. It's discussed elsewhere on Community.

> I could not find any definitions for this Callback function!.

My changed stm32h7xx_hal_eth.h file needs to be placed in an include directory your compiler searches before the "Drivers/STM32H7xx_HAL_Driver/Inc" directory.

> I am not going to use LWIP!

Assuming you're not meaning you're using a different stack, you'll need some way to queue packets where a packet may be a chains of buffers. Even bare-metal you'd implement queues. Even if you processed received packets immediately and discarded you'd still need queues... because your list of unused buffers is a queue. LwIP does it with struct pbuf. You'd need it for rx and tx. So your struct would need members for at least buffer length and next. For RxPoolInit, link all your buffers onto your free list. For RxBufferAlloc, allocate one buffer from your free list. For RxBuffFree, return one buffer to your free list. For RxPktAssemble, chain buffers by their next. For RxPktDiscard, return each buffer of a chain to your free list.

[Anand Ram]

hi @alister​  ,

thank you.

unfortunately the solution doesnot works and breaks at below.

 /* call user specified initialization function for netif */

 if (init(netif) != ERR_OK) {

  return NULL;

 }

can somebody please share the references of MX_LWIP_Init issue?

may thanks in advance

[alister]

> can somebody please share the references of MX_LWIP_Init issue?

MX_LWIP_Init hasn't changed between H7_FW V1.5.0 and H7_FW V1.6.0. It uses lwIP's raw API and you have to take care it's executed by only one thread at a time.

In H7_FW V1.5.0 it's called after FreeRTOS' scheduler starts, which could be unsafe.

In H7_FW V1.6.0 it's called before FreeRTOS' scheduler starts. So its using the lwIP raw API should be ok.

Reading MX_LWIP_Init, we see it sets the interface up or down based on whether the link is up or down. This looks like a bug. Piranha mentions it too at https://community.st.com/s/question/0D50X0000BOtfhnSQB/how-to-make-ethernet-and-lwip-working-on-stm32. It requires the init arg to netif_add, ethernetif_init in ethernetif.c to set the link status bit in the netif flags.

My opinion is you should flag the interface up as soon as you've initialised it, start with the link down, and flag the link up and down as you observe it change. This is in your phy code.

I've never used or debugged MX_LWIP_Init.

It's hardwired to use DHCP, whereas my app doesn't know whether to or not until later.

Perhaps it works. Not sure. Just it looks wrong.

> unfortunately the solution doesnot works and breaks at below.

The init arg to netif_add is ethernetif_init, and it's not returning ERR_OK or it's crashing.

Reading ethernetif_init, we see it can only return ERR_OK. So it's crashing somewhere in low_level_init.

You'll need to step the code to find why.

[Ciuffoly www.audiodesignguide]

Hello, I would like to add ethernet support to my TouchGFX application.

I have got the lwIF directory from Cube and insert in TouchGFX Makefile with:

components := TouchGFX/gui target TouchGFX/generated/gui_generated LwIP

but I got many errors about include location, after many changes I have now this error if I use this define:

IP4_ADDR(&netmask, 255,255,255,255);:

Drivers/BSP/STM32746G-Discovery/stm32746g_discovery.c: In function 'udp_echo_init':

       LwIP/include/lwip/ip4_addr.h:120:44: error: 'struct ip_addr' has no member named 'addr'; did you mean 'u_addr'?

        #define IP4_ADDR(ipaddr, a,b,c,d) (ipaddr)->addr = PP_HTONL(LWIP_MAKEU32(a,b,c,d))