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

[Feature Request] Updating Portable NetworkInterface & Drivers for STM32 #766

Closed
HTRamsey opened this issue Mar 9, 2023 · 5 comments
Closed
Labels
enhancement New feature or request

Comments

@HTRamsey
Copy link
Contributor

HTRamsey commented Mar 9, 2023

Is your feature request related to a problem? Please describe.
I'd like to update the STM32F & H based network drivers and do some work to make them cleaner, newer, more efficient, etc. The STM32F based NetworkInterface lags behind the H based one and could be brought up to par. It is also based on a driver from F4 1.7.0 which is very old now.

Describe the solution you'd like
To update based on newer HAL drivers, create common file, etc. A common STM32 file would make future updates much easier, and is feasible because newer HAL drivers are extremely similar.

Describe alternatives you've considered
Doing nothing

Additional context
I'm happy to do it myself, just checking if this would be acceptable to be merged later on. I believe @htibosch is the one to approach about this.

@HTRamsey HTRamsey changed the title [Feature Request] <Updating Portable NetworkInterface & Drivers for STM32> [Feature Request] Updating Portable NetworkInterface & Drivers for STM32 Mar 9, 2023
@paulbartell
Copy link
Contributor

Hey there @holden-zenith . We're happy to take your contribution, just open a PR. @AniruddhaKanhere and @htibosch would be the best people to discuss design plans with.

@HTRamsey
Copy link
Contributor Author

@AniruddhaKanhere @htibosch I have a working NetworkInterface for STM32F7 based on the recent reworked ST drivers. Should I post a draft PR to discuss updating the one in the repo? A couple of things I think should be done:

  • Current Fxx file includes logic for F1, F2, F4, & F7. F1 & F2 don't seem to be getting the new drivers. So, perhaps note the current NetworkInterface as legacy but leave it for those using F1 or F2 and create a new updated interface for F4 & F7. The hal_eth files will now be identical for F4 & F7.
  • Reworked drivers are phy independent, perhaps it is possible to just use the native driver file without edits.
  • Update the Fxx interface to include some of the newer implementation features included in the newer interfaces like the Hxx.
  • H5 has been released, needed changes to Hxx interface?
  • There is quite a lot of common code between the STM32 implementations. Consider making a common STM file to clean up & make updates easier?
  • Bring Hxx driver up to date as well
  • Any other basic documentation, code quality, readability, safety updates I come across

@paulbartell paulbartell added the enhancement New feature or request label Mar 13, 2023
@AniruddhaKanhere
Copy link
Member

Hello @holden-zenith,

I think a draft PR would be the way to go. That would allow all of us to discuss the changes and additions while allowing us to clone your changes and test them out locally.

The points that you noted above do make sense to me. We can discuss more on how to deal with legacy drivers for F1 and F2 once you create the draft PR. It will help us visualize how different everything looks and whether that is good for backwards compatibility.

Does that make sense? What do you think @htibosch?

- Aniruddha

@htibosch
Copy link
Contributor

I have a working NetworkInterface for STM32F7 based on the recent reworked ST drivers

Yes an update (/upgrade) would be very welcome. I wrote the STM43F4x driver many years ago. Since then we only added more subtypes (F1, F2, and F7).

I assume that you new version is also "zero-copy", and that it is also aware of cached memory?

Looking forward to see your new version.

Thanks

@tony-josi-aws
Copy link
Member

Closing this issue as the related PR #804 has been merged.

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

No branches or pull requests

5 participants