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 KSZ8081 support. #5061

Merged
merged 1 commit into from
Apr 15, 2021
Merged

Add KSZ8081 support. #5061

merged 1 commit into from
Apr 15, 2021

Conversation

rtrbt
Copy link
Contributor

@rtrbt rtrbt commented Apr 14, 2021

This adds support for the KSZ8081 ethernet phy.
Only the IDF 4+ specific code is modified, as the phy support was only
added recently:
espressif/esp-idf@aecfbf96

This adds support for the KSZ8081 ethernet phy.
Only the IDF 4+ specific code is modified, as the phy support was only
added recently:
espressif/esp-idf@aecfbf96
@me-no-dev me-no-dev merged commit e6ba8c7 into espressif:master Apr 15, 2021
@me-no-dev
Copy link
Member

Thanks :)

@brandonros
Copy link

brandonros commented Apr 21, 2021

CXX build/arduino/libraries/WiFi/src//ETH.o
/Users/brandonros/Desktop/esp-idf-v4.3/examples/get-started/sample_project/components/arduino/libraries/WiFi/src/ETH.cpp: In member function 'bool ETHClass::begin(uint8_t, int, int, int, eth_phy_type_t)':
/Users/brandonros/Desktop/esp-idf-v4.3/examples/get-started/sample_project/components/arduino/libraries/WiFi/src/ETH.cpp:174:23: error: 'esp_eth_phy_new_ksz8081' was not declared in this scope
             eth_phy = esp_eth_phy_new_ksz8081(&phy_config);
                       ^~~~~~~~~~~~~~~~~~~~~~~
/Users/brandonros/Desktop/esp-idf-v4.3/examples/get-started/sample_project/components/arduino/libraries/WiFi/src/ETH.cpp:174:23: note: suggested alternative: 'esp_eth_phy_new_ksz8041'
             eth_phy = esp_eth_phy_new_ksz8081(&phy_config);
                       ^~~~~~~~~~~~~~~~~~~~~~~
                       esp_eth_phy_new_ksz8041
make[1]: *** [libraries/WiFi/src//ETH.o] Error 1
make: *** [component-arduino-build] Error 2

I'm pretty sure this broke build against https://github.com/espressif/esp-idf.git release/v4.3

@rtrbt any recommendations?

Edit: I figured it out. git cherry-pick 98a4c70af94acdfb93baf48d7d25fe4d1560e4df against esp-idf

@jesse-schein
Copy link

Yea this PR broke esp-idf 4.3 that platformio uses

@rtrbt
Copy link
Contributor Author

rtrbt commented Aug 23, 2021

Building with pio works if you use the idf-master branch of the platform package like this:
platform = https://github.com/platformio/platform-espressif32.git#3b5de56
or
platform = https://github.com/platformio/platform-espressif32.git#idf-master

I could also add a version check to the ksz8081 specific code, so it is only compiled in if ESP-IDF 4.4 is available. But I'm not sure if this is a good idea, because a quick grep did not find any usage of IDF_MINOR or ESP_IDF_VERSION_MINOR (assuming this isn't the only occurence of code that does require a higher version than 4.0). @me-no-dev: Any thoughts?

@jesse-schein
Copy link

Building with pio works if you use the idf-master branch of the platform package like this:
platform = https://github.com/platformio/platform-espressif32.git#3b5de56
or
platform = https://github.com/platformio/platform-espressif32.git#idf-master

I could also add a version check to the ksz8081 specific code, so it is only compiled in if ESP-IDF 4.4 is available. But I'm not sure if this is a good idea, because a quick grep did not find any usage of IDF_MINOR or ESP_IDF_VERSION_MINOR (assuming this isn't the only occurence of code that does require a higher version than 4.0). @me-no-dev: Any thoughts?

Sounds to me like this is more of a platformio issue then. I was just mentioning it as a comment in case anyone else got stuck with the build errors and wanted some information on why the error was occuring if using platformio.

@LordRafa
Copy link

LordRafa commented Sep 9, 2021

Building with pio works if you use the idf-master branch of the platform package like this:
platform = https://github.com/platformio/platform-espressif32.git#3b5de56
or
platform = https://github.com/platformio/platform-espressif32.git#idf-master
I could also add a version check to the ksz8081 specific code, so it is only compiled in if ESP-IDF 4.4 is available. But I'm not sure if this is a good idea, because a quick grep did not find any usage of IDF_MINOR or ESP_IDF_VERSION_MINOR (assuming this isn't the only occurence of code that does require a higher version than 4.0). @me-no-dev: Any thoughts?

Sounds to me like this is more of a platformio issue then. I was just mentioning it as a comment in case anyone else got stuck with the build errors and wanted some information on why the error was occuring if using platformio.

This PR also broke VSCode official plug-in and idf.py... if you go IDF official github, you can see that the function esp_eth_phy_new_ksz8081 is missing on the current stable branch 4.3 (so nothing to do with platformio IMHO).

Lets hope that 4.4 fix this in the mean time commenting out this PR modification does the trick, hopefully should be harmless unless you want to use the ksz8081 IC for Ethernet (which is not supported on 4.3 anyway)....

@rtrbt
Copy link
Contributor Author

rtrbt commented Sep 9, 2021

FYI: #5599

@LordRafa
Copy link

LordRafa commented Sep 9, 2021

Cool anyway version 3.4.1 was just released with the function esp_eth_phy_new_ksz8081 in place

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.

5 participants