Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add emac section #260

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

Add emac section #260

wants to merge 3 commits into from

Conversation

JojoS62
Copy link

@JojoS62 JojoS62 commented Mar 12, 2024

Summary of changes

This PR adds a section for emac variables. It is necessary to place them into a location that can be specified in the linker script. For devices with cache like the STM32H7 this is essential, the network will fail after a short time when the emac vars are in a cached RAM area.

Example for linker settings:

.emac_section  (NOLOAD) : { *(.DMA_EMAC_section) } >SRAM2

Impact of changes

This modification has no impact when the linker section is not defined. There will be no warning or error thrown when the section does not exist, the linker will place the variables where it wants.
So to use this section, check the map file for the correct placement.

Migration actions required

Documentation


Pull request type

[] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

[] No Tests required for this change (E.g docs only update)
[] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

Reviewers


@multiplemonomials
Copy link
Collaborator

Should we update the STM32H7 linker scripts to have this section?

@JojoS62
Copy link
Author

JojoS62 commented Mar 14, 2024

There are different RAM areas in the H7 variants, and the setting is only necessary when Ethernet is used. That is why I wanted to have the custom linker scripts, these can handle the different demands better.
I would rather add an Ethernet example to the new library repo.

@multiplemonomials
Copy link
Collaborator

Eh, but... it's a big issue if Ethernet doesn't work out of the box on STM32H7 boards... like I think this guy was just having trouble last week https://forums.mbed.com/t/mbed-ce-ethernet-nucleo-h723-not-working/22566

@JojoS62
Copy link
Author

JojoS62 commented Mar 14, 2024

ok, I will check it. But I can't test it on the H723.
But the error with the missing ping reply could have some other reason.

@JohnK1987
Copy link
Member

I do not understand this issue very well but I am using STM32H743ZI2 as a heart of Tester in my project. The H743ZI2 is connected over Ethernet and communicate via UDP with my PC app. This tester was able to run for few weeks without any issue on Mbed side.

@multiplemonomials
Copy link
Collaborator

Yes at USC RPL we have also used ethernet on H743 for some time with no issues... have we just been getting lucky?

@JojoS62
Copy link
Author

JojoS62 commented Mar 14, 2024

I can reproduce the problem very reliable. I have a MQTT client running and it fails after a few minutes. With this fix, it runs also for weeks.

@JojoS62 JojoS62 marked this pull request as draft March 14, 2024 09:56
@JojoS62
Copy link
Author

JojoS62 commented Mar 14, 2024

ok, I have double checked. I was confused with H7/F7, there are some H7 targets where the linker script has sections for RxDecripSection/TxDescripSection. E.g. the H743 has this sections, so this target is ok, the H723 doesn't have it.

Also the F769 and F746 don't have the linker script settings and there I have the problems. I must check again, it is also possible that the problems come from bad alignement, the descriptors need to have 32 Byte alignment.
Modifying the scripts has an impact on existing software, the RAM is used completely in one block. Changes have an impact on the heap size, specially when Ethernet is not used.

The STM Emac Code has two versions, the H7 uses a different than the others. The correct alignment is only implemented for the Verstion ETH_IP_VERSION_V2 or H7. For the other targets, the alignment is set to 4 Byte and this is not correct for the descriptor table.

@multiplemonomials
Copy link
Collaborator

I started digging into this a bit on my end. On NUCLEO_H723ZG, Ethernet does not work reliably. It cannot pass the tests because the tests try to send a data buffer allocated with malloc, and on H723ZG, malloc can allocate from DTCM. The Eth DMA cannot access the DTCM memory, so it dies.

In parallel, there's also a bit of a mess going on with the linker script setup for the Ethernet buffers. It's really not pretty, as it relies on extremely large reserved areas of memory to make things work, and it does so in a very non obvious way.

@multiplemonomials
Copy link
Collaborator

I think that one fix we need is, the STM32 EMAC driver needs to be able to reallocate a pbuf from the heap if it receives a Tx buffer that is in an inaccessible memory area.

@multiplemonomials
Copy link
Collaborator

Update: I was able to fix this locally, but it will require a significant refactor to the STM32H7 linker scripts before I can actually merge this fix, because things were hella broken.

@multiplemonomials
Copy link
Collaborator

Pushed up my changes here but still need to refactor all the linker scripts. https://github.com/mbed-ce/mbed-os/commits/feature/stm32h7-emac-rewrite/

@multiplemonomials
Copy link
Collaborator

#356 takes care of the linker script issues here, so now it's actually possible to work on the Ethernet MAC issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants