-
-
Notifications
You must be signed in to change notification settings - Fork 179
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 ethernet driver for M5Core2 #3085
base: main
Are you sure you want to change the base?
Conversation
Add ethernet driver to CMakePresets
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe pull request modifies the Changes
Possibly related PRs
Suggested labels
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
targets/ESP32/CMakePresets.json (1)
487-487
: Consider adding documentation for Ethernet configurations.To improve maintainability and help future contributors, consider adding comments explaining:
- The supported PHY chips and their configurations
- The required GPIO pins and their purposes
- References to hardware documentation
Example documentation to add at the top of the file:
+# Ethernet Configuration Guide +# --------------------------- +# ESP32_ETHERNET_SUPPORT: Enable Ethernet support +# ESP32_ETHERNET_INTERFACE: PHY chip (supported: LAN8720, IP101, RTL8201) +# ETH_PHY_RST_GPIO: Reset pin for PHY chip +# ETH_RMII_CLK_OUT_GPIO: RMII clock output pin +# ETH_RMII_CLK_IN_GPIO: RMII clock input pin (alternative to _OUT_) +# ETH_PHY_ADDR: PHY chip address (usually 0 or 1) +# +# For more details, see: <link-to-docs>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
targets/ESP32/CMakePresets.json
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: nf-interpreter (Check_Code_Style)
- GitHub Check: nf-interpreter (Nightly build) (Check_Build_Options)
- GitHub Check: nf-interpreter (Check_Build_Options)
🔇 Additional comments (1)
targets/ESP32/CMakePresets.json (1)
487-487
: Verify the Ethernet module type and interface.The PR description mentions LAN-W5500 derivatives, but the configuration suggests standard RMII Ethernet. The W5500 typically uses SPI interface, not RMII.
Please clarify:
- Is the target module a W5500-based board or a standard RMII Ethernet module?
- If it's W5500:
- Should we use SPI interface instead?
- Do we need different GPIO configurations?
- If it's RMII:
- Which PHY chip is being used?
- What are the correct GPIO pins?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
targets/ESP32/CMakePresets.json
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: nf-interpreter (Check_Code_Style)
- GitHub Check: nf-interpreter (Nightly build) (Check_Build_Options)
- GitHub Check: nf-interpreter (Check_Build_Options)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
targets/ESP32/CMakePresets.json (1)
487-488
: Add SPI host configuration for W5500.Consider adding the SPI host configuration to specify which SPI peripheral the W5500 module should use:
"ESP32_ETHERNET_SUPPORT": "ON", "ESP32_ETHERNET_INTERFACE": "W5500", +"ESP32_ETHERNET_SPI_HOST": "SPI2_HOST",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
targets/ESP32/CMakePresets.json
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: nf-interpreter (Build_ESP32_targets ESP32_ETHERNET_KIT_1.2)
- GitHub Check: nf-interpreter (Build_ESP32_targets ESP32_S3_ALL)
- GitHub Check: nf-interpreter (Build_ESP32_targets ESP32_H2_THREAD)
- GitHub Check: nf-interpreter (Build_ESP32_targets ESP32_C6_THREAD)
- GitHub Check: nf-interpreter (Build_ESP32_targets ESP32_C3)
- GitHub Check: nf-interpreter (Build_ESP32_targets ESP32_BLE_REV0)
- GitHub Check: nf-interpreter (Build_ESP32_targets ESP32_PSRAM_REV0)
- GitHub Check: nf-interpreter (Check_Code_Style)
- GitHub Check: nf-interpreter (Nightly build) (Check_Build_Options)
- GitHub Check: nf-interpreter (Check_Build_Options)
🔇 Additional comments (1)
targets/ESP32/CMakePresets.json (1)
487-494
: Verify GPIO pin assignments for M5Core2 W5500 module.The Ethernet configuration for W5500 looks good with all required SPI configurations. However, please verify that the GPIO pin assignments match your M5Core2's W5500 module hardware layout:
- MISO: GPIO19
- MOSI: GPIO23
- SCLK: GPIO18
- INT: GPIO34
- CS: GPIO26
- RST: GPIO13
I'm not able to send a commit, but here are some changes I did in NF_ESP32_Ethernet.cpp : Moved eth_esp32_emac_config_t esp32_emac_config = ETH_ESP32_EMAC_DEFAULT_CONFIG(); to '#if ESP32_ETHERNET_INTERNAL == TRUE As I understand this doesn't apply to SPI Ethernet. Changed all I guess a variable ESP32_ETHERNET_SPI_MODE would be welcome. Everything is compiling, I've seen references to W5500 in the nanoCLR.bin file, but when running |
Description
This PR adds the required
"ESP32_ETHERNET_SUPPORT": "ON",
to CMakePresets.json so that it is possible to use them for M5Core2.Currently needs to consider support dependent on actual boad, e.g.
M5Core2 for AWS seems to have a different pin map (when using the compatibility calculator in https://docs.m5stack.com/en/base/lan_poe_v12 ):
Motivation and Context
The M5Core2 supports ethernet through modules including the LAN-W5500 derivatives:
How Has This Been Tested?
Screenshots
Types of changes
Checklist
Summary by CodeRabbit