8000 Fix invalid clock config for ethernet chip on STM32 F-series MCU by ukicomputers · Pull Request #13422 · micropython/micropython · GitHub
[go: up one dir, main page]

Skip to content

Fix invalid clock config for ethernet chip on STM32 F-series MCU #13422

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

Closed
wants to merge 10 commits into from

Conversation

ukicomputers
Copy link

Tested on STM32F437ZG with LAN8720A. Valid for F7, tested on F4 series, usable on others. Check discussion #13406 for more info about problem. May required fixing this problem on H-series! (check updated lines)

Tested on STM32F437ZG with LAN8720A. Valid for F7, tested on F4 series, usable on others. Check discussion micropython#13406 for more info about problem. May required fixing this problem on H-series! (check updated lines)

Signed-off-by: Uglješa Lukešević <84191191+ukicomputers@users.noreply.github.com>
…d add required speed for communication pins.

Signed-off-by: Uglješa Lukešević <84191191+ukicomputers@users.noreply.github.com>
@robert-hh
Copy link
Contributor

Some comments:

  • Not all pins have to be configured for VERY_HIGH_SPEED. It is required only for MICROPY_HW_ETH_RMII_TXD0 and MICROPY_HW_ETH_RMII_TXD1, if at all.
  • MCO output vie HAL_RCC_MCOConfig() should not enabled for all boards. If a specific board hardware requires this, there must be a set of board files for this board, and enabling MCO can go into a board_init.c file with a board_early_init() function.

@ukicomputers
Copy link
Author
ukicomputers commented Feb 11, 2024

@robert-hh I will change the pin configuration. For the MCO clock, should I add a flag if someone wants to use it like: MICROPY_HW_ETH_RMII_ENABLE_CLOCK, or just include it in board_init.c as you said?

@ukicomputers ukicomputers marked this pull request as draft February 11, 2024 13:54
@robert-hh
Copy link
Contributor

Better put it into a board_init.c file, because it is not strictly related to LAN. But if it is in eth.c,. call the different, like
MICROPY_HW_ETH_RMII_CLK_OUT. In any way, this hardware design is an unfortunate one, because it ties ETH to a specific MCU clock setting.

Signed-off-by: Uglješa Lukešević <84191191+ukicomputers@users.noreply.github.com>
Add default flag for outputting clock value. 

Signed-off-by: Uglješa Lukešević <84191191+ukicomputers@users.noreply.github.com>
Signed-off-by: Uglješa Lukešević <84191191+ukicomputers@users.noreply.github.com>
@ukicomputers
Copy link
Author

Got it. I think better to put it as #define, because if any newbie comes, it can try changing some settings for ETH, not to go through different board ports without valid explanation comment. Things changed:

  • Use MICROPY_HW_ETH_RMII_CLK_OUT to setup MCO config
  • Change GPIO speed configs to reasonable speeds

Don't yet merge this PR because it still needs to be tested.

@robert-hh
Copy link
Contributor

You can set a PR top draft mode. Then it will not be merged. Anyhow, merging takes a while.
You forgot the #if MICROPY_HW_ETH_RMII_CLK_OUT around the statement enabling the MCO signal. And you will still need board files for your board set-up.

@ukicomputers
Copy link
Author

Sorry, what do you mean? I added it with stm32/eth.c: Use CLK_OUT flag.. Only if you are thinking for something related to mpconfigport.h.

@robert-hh
Copy link
Contributor

Now I see it. At the time of writing, it was not visible yet.

@ukicomputers ukicomputers marked this pull request as ready for review February 12, 2024 12:03
@ukicomputers ukicomputers marked this pull request as draft February 12, 2024 12:04
Signed-off-by: Uglješa Lukešević <84191191+ukicomputers@users.noreply.github.com>
Signed-off-by: Uglješa Lukešević <84191191+ukicomputers@users.noreply.github.com>
@ukicomputers
Copy link
Author

Conclusion for this PR

  • The external clock is not used somewhere on the hardware designs. To get MCU to provide clock, in definitions you need to enable MICROPY_HW_ETH_RMII_CLK_OUT. The value of that definition is defaulting to 0: no providing clock.
  • The STM32 MCU's are very sensitive to clock config, so recommended is to check them twice. The clock defined is for one peripheral, and it is different to Ethernet, USB, CAN...
  • This debunking credit goes to @robert-hh. Thanks!
  • The discussion Ethernet LAN8720A does not work on MicroPython port for STM32 #13406 should clear everything up.

@ukicomputers ukicomputers marked this pull request as ready for review February 13, 2024 10:48
@robert-hh
Copy link
Contributor
  • About the change of the REF_CLK pin. This is an input. So the speed setting has no effect.
  • When the speed high or VERY_HIGH is enabled, the data sheet recommends to enable the compensation cell as well. But you should check if the speed MEDIUM is not already sufficient. In my test set-up it works, and the EM radiation is definitely lower.
  • You should make for your specific board combination a set of board files, including a board_init.c file for the set-up. The LAN hardware you use is a somewhat "rare" design, creating a dependency between CPU clock and Ethernet clock. All other LAN87xx uses I know have a dedicated 50MHz clock generator for RefClk.

Signed-off-by: Uglješa Lukešević <84191191+ukicomputers@users.noreply.github.com>
@dpgeorge
Copy link
Member
dpgeorge commented Mar 7, 2024

Now that #13630 has been merged, I'm not sure how much of this PR is needed.

If parts of this PR are needed then please rebase on the latest master.

@ukicomputers
Copy link
Author
ukicomputers commented Mar 7, 2024

After @robert-hh commited, most of the issues are resolved. Only specific hardware design like mine requires setting clock output. If someone needs CLK_OUT set up, it should be defined in board_init.c

< 8000 div class="js-timeline-item js-timeline-progressive-focus-container" data-gid="CE_lADOAOoGts58AT2BzwAAAALPAAC0">
@ukicomputers
Copy link
Author

Also, if someone needs it defined in MPY board.h file, it can just use my fork of MPY.

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

Successfully merging this pull request may close these issues.

3 participants
BBB
0