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

First implementation of Ethernet for STM32F7 (and later STM32F4). #466

Merged
merged 5 commits into from
Feb 18, 2021

Conversation

mikewolfram
Copy link
Contributor

Et voila, here we go. This is the code I currently use plus an example for F767ZI Nucleo board. The example is a quick hack of code I already use.

Quite a few things to do, e.g. the module.lb file is missing. Also the network interface file for FreeRTOS might need a better location. And in it there is hardcoded LAN8720 PHY. I was thinking to make that an option in the XML or required the use to create an include file with the proper declaration.

Let's start from here, any help is appreciated. I'm a bit short of time until the end of the year, but together we should be able to get it done.

@mikewolfram
Copy link
Contributor Author

Wanted to make it a draft PR, any way to change it? Or delete and recreate?

@strongly-typed
Copy link
Collaborator

ERROR: Cannot resolve name 'modm:platform:eth' of type Module!

Could you please also make your modm module available? Thank you!

@mikewolfram
Copy link
Contributor Author

Could you please also make your modm module available? Thank you!

As of now I don't have a module file. Before opening the PR I had those files in my sources, I copied them to that location for the PR. So even with a module file I expect them to not compile as the includes etc. won't work.

Sorry for that. Right now I'm hacking together (is this real English? :-) ) a proof of concept while throwing away whatever I learned about software development, coding style etc.

Working 11-12 hours a day, 11 days in a row and living in a hotel isn't so much fun. :-( Let's see what I could for you...

@mikewolfram
Copy link
Contributor Author

And without a separate project I cannot event run "lbuild" as it would overwrite changes I made to some files. :-( I know, I completely messed it up.

@salkinium
Copy link
Member

salkinium commented Sep 16, 2020

Please don't burn yourself out!

(Sascha is also capable of adding a suitable module.lb himself *wags finger like a mom*)

@mikewolfram
Copy link
Contributor Author

mikewolfram commented Sep 16, 2020

Please don't burn yourself out!

I'm already beyond a burnout... :-)

(Sascha is also capable of adding a suitable module.lb himself wags finger like a mom)

Sure he can, but he's afraid to destroy my genius work. :-)
(Sorry, couldn't resist).

@strongly-typed
Copy link
Collaborator

Take your time! I am just curious in bringing Ethernet support to modm. I have no deadlines nor customers involved with this topic. I have a Nucleo F429ZI (which claims to have Ethernet) for further testing and maybe I can help with your current memory problems.

@mikewolfram
Copy link
Contributor Author

I have a few days off and won't be touching any keyboard...

The F4 series might be a bit different to the F7 in regards to ethernet. I used the FreeRTOS network interface for STM32 HAL as example and stripped off things. There are a few places where they differ between F4 and F7. They also provide a modified version of the HAL_ETH files. Maybe you want to have a look to get an idea what is the actual difference.

More to come next week.

@mikewolfram
Copy link
Contributor Author

I have updated with the latest from develop, don't know why all the commits are showing up here now. Did I screw something up?

Still investigating the memory issue. I have a feeling, that the stack of the IPTask might get corrupted. In my driver I send an event with the network descriptor pointer, which is always in the range 0x20013000 to 0x20015000. Had a hard fault, where the received event had a pointer of 0x2002ba24. Increased the stack from 640 to 1024 words, let's see if that changes anything.

The other tasks in theory should not be able to modify the IPTask stack, but you never know. I have a very simple task that only toggles a LED every second, even that one needs more than the minimal 128 words for the stack. Set it to 256 words and it is now running.

@mikewolfram
Copy link
Contributor Author

And another question. I saw that you moved a few files in FreeRTOS_TCP. When I added it I had a rather simple directory layout, but maybe it would make sense to move the headers into a freertos_tcp folder like freertos?

@salkinium
Copy link
Member

Delegating to @strongly-typed.

@mikewolfram
Copy link
Contributor Author

Short question, one example is failing, because it is complaining about a symbol defined in stm32f765xx.h. I thought this would be included before? Or did I miss something?

@strongly-typed
Copy link
Collaborator

And another question. I saw that you moved a few files in FreeRTOS_TCP. When I added it I had a rather simple directory layout, but maybe it would make sense to move the headers into a freertos_tcp folder like freertos?

I had duplicated includes so I only copied the needed files. Yes, a subdirectory would make perfect sense.

(I do not know if there are any dependencies at FreeRTOS that require the files to be in one directory.)

@mikewolfram
Copy link
Contributor Author

Looks like the code is running fine now. Maybe the fix to drop invalid frames was the issue? That would explain why it was working fine on an isolated network, but crashed on a busy network in the office?

@mikewolfram mikewolfram mentioned this pull request Jan 29, 2021
4 tasks
@mikewolfram mikewolfram marked this pull request as ready for review February 3, 2021 23:08
@mikewolfram
Copy link
Contributor Author

mikewolfram commented Feb 3, 2021

I wasn't aware this is still a draft and was wondering why we didn't get any further. I turned the status to "Ready for review", so let's discuss what is needed to finally merge it (e.g. lack of documentation).

We have the code running for several months now, with no major problems (still don't know if this is a bug in the implementation or the result of some crazy network setup in the company).

@rleh
Copy link
Member

rleh commented Feb 4, 2021

I don't know what happened with the MacOS CI, but it is now fixed (by re-running the job).
I will do a review later today!

Copy link
Member

@rleh rleh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made a few minimal changes in 10a4cff (remove commented-out code and inline some static variables), otherwise looks very good!

@rleh
Copy link
Member

rleh commented Feb 11, 2021

I can confirm, that

  • the blink/-example works on hardware
  • the ethernet/-example does not work. 😒

The logger just outputs Reboot: Ethernet Example and then nothing happens. The LEDs on the ethernet jack light up once I plug in a cable.

@mikewolfram
Copy link
Contributor Author

Thanks for testing. Let's find out what is different to my custom board and what might be wrong.

Did you check with the debugger what is causing the reboot? Maybe the stack needs to be increased?

@rleh
Copy link
Member

rleh commented Feb 11, 2021

For some reason GDB does not work on my computer, i'm working on it...

Compiling in debug mode (scons profile=debug) tells me the following:

Reboot: Ethernet Example
Assertion 'eth.init' @ 0x00000001 (1) failed!
  ETH::initialize() timed out on software reset!
Abandoning...

Copy link
Member

@rleh rleh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Successfully tested on hardware.

Thanks!!

@rleh
Copy link
Member

rleh commented Feb 11, 2021

Ping and www are good?

No. 😒
I was a bit hasty and accidentally pinged another device on the network.

@rleh
Copy link
Member

rleh commented Feb 11, 2021

Code runs until

while (true) {
  connectedSocket = FreeRTOS_accept(listeningSocket, &clientAddress, 0);
//...

but never seems to receive a connection.

I'll look at this further later.

@kikass13
Copy link

ping and http work for me on a nucleo F767ZI, after correcting two pins for the phy.

using RMII_Tx_En = GpioOutputB11;
using RMII_Tx_D0 = GpioOutputB12;
  • to
using RMII_Tx_En = GpioOutputG11;
using RMII_Tx_D0 = GpioOutputG13;

@rleh
Copy link
Member

rleh commented Feb 18, 2021

I applied @kikass13 fix, syncronized the docs and rebased onto current develop.
Thanks @kikass13 for finding the error!

@salkinium Are you ok with merging (after CI has finished)?

Copy link
Member

@rleh rleh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested: Ping and HTTP work ✔️

Copy link
Member

@salkinium salkinium left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚢 it

@rleh rleh merged commit d8be0a2 into modm-io:develop Feb 18, 2021
@salkinium
Copy link
Member

salkinium commented Feb 18, 2021

Something in this PR broke the docs generator. *wags finger*

Generating documentation for stm32f207zgt7 ...
Error generating documentation for device stm32f207zgt7: 'NoneType' object is not subscriptable

@kikass13
Copy link

Something in this PR broke the docs generator. wags finger

Generating documentation for stm32f207zgt7 ...
Error generating documentation for device stm32f207zgt7: 'NoneType' object is not subscriptable

someone was using a list iterator on an object without checking for None ... SHAME! :D

@salkinium
Copy link
Member

I think… *takes a long bong hit*… we need a CI for our CI test coverage…

@rleh
Copy link
Member

rleh commented Feb 18, 2021

Something in this PR broke the docs generator. wags finger
Generating documentation for stm32f207zgt7 ...

... but 15 times, spread random across multiple STM32 sub-families.

And the test run of the docs.modm.io script was successful. Maybe related to #553?

@salkinium
Copy link
Member

It's probably something stupid, I'll check it out later.

@rleh
Copy link
Member

rleh commented Feb 21, 2021

It's probably something stupid, I'll check it out later.

Did you find out anything?
I just restarted the last successful build on CircleCI (without ethernet merged) to fix the 404 errors on https://docs.modm.io/...

@salkinium
Copy link
Member

I forgot, but it was something very stupid, so I found it very quickly: #565

Comment on lines +59 to +62
// initialize random numbers
time_t now;
time(&now);
ulNextRand = uint32_t(now);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could have used real random numbers here...

Suggested change
// initialize random numbers
time_t now;
time(&now);
ulNextRand = uint32_t(now);
// initialize random numbers
modm::platform::RandomNumberGenerator::enable();
while(!modm::platform::RandomNumberGenerator::isReady()) {}
ulNextRand = modm::platform::RandomNumberGenerator::getValue();

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, and as a fallback you can seed srand with the device's UID.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also time() is not implemented. We could perhaps wire it to modm::Clock, but it's not done yet.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, and as a fallback you can seed srand with the device's UID.

I am not sure if this is a good idea or a serious security flaw...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, you need to collect entropy somehow. There are a bunch more hardware calibration registers that provide entropy within some bounds. It's a somewhat good idea since the values will be different for every device (assuming a good hash function), however, only if the attacker cannot read them out remotely. However, security is definitely beyond the scope of modm and too much work to implement yourself. We'd have to integrate some embedded TLS library for encryption.

@rleh rleh mentioned this pull request Apr 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

6 participants