Skip to content

drivers: eth_esp32: allow selecting ref clk source #65759

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

Merged

Conversation

bbilas
Copy link
Collaborator

@bbilas bbilas commented Nov 25, 2023

In case of boards where REF_CLK signal is not connected to the GPIO0 by default add the possibility to use the optional GPIO16/GPIO17 as a REF CLK source.

@zephyrbot zephyrbot added platform: ESP32 Espressif ESP32 area: Ethernet area: Devicetree Binding PR modifies or adds a Device Tree binding labels Nov 25, 2023
@bbilas bbilas force-pushed the esp32-eth-gpio-ref-clk-output branch from de31a03 to 3b17111 Compare November 25, 2023 13:43
@bbilas
Copy link
Collaborator Author

bbilas commented Nov 25, 2023

@sylvioalves compliance failure is related to the typo within espressif HAL, how should I fix that?

Comment on lines 25 to 31
ref_clk_gpio_output:
type: int
enum:
- 16
- 17
description: |
GPIO number to output RMII Clock.
Copy link
Member

Choose a reason for hiding this comment

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

How about using standard phandle-array for gpios? e.g. ref-clk-output-gpios = <&gpio0 16 0>;

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't see benefits from it since this is not handled by the Zephyr GPIO API but the espressif HAL directly. phandle-array would not be also possible to validate by the YAML parser itself as it is done right now.

Copy link
Member

Choose a reason for hiding this comment

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

This is about having single way to describe GPIOs in devicetree files, regardless how those are used by driver (and whether Zephyr GPIO API is used or not).

See nordic,nrf-led-matrix.yaml bindings, which uses standard gpios phandles for rows and columns, but internally in the driver just nRF specific representation of GPIOs are used.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fair enough, I've changed that into phandle-array with additional build asserts.

@bbilas
Copy link
Collaborator Author

bbilas commented Nov 25, 2023

@sylvioalves compliance failure is related to the typo within espressif HAL, how should I fix that?

zephyrproject-rtos/hal_espressif#250

@bbilas bbilas force-pushed the esp32-eth-gpio-ref-clk-output branch from 3b17111 to fd5ac18 Compare November 25, 2023 15:26
@bbilas bbilas force-pushed the esp32-eth-gpio-ref-clk-output branch from fd5ac18 to 03afad5 Compare November 25, 2023 15:50
mniestroj
mniestroj previously approved these changes Nov 25, 2023
@bbilas
Copy link
Collaborator Author

bbilas commented Nov 26, 2023

Bumped hal_espressif to fix the compliance failure.

@zephyrbot
Copy link
Collaborator

zephyrbot commented Nov 26, 2023

The following west manifest projects have been modified in this Pull Request:

Name Old Revision New Revision Diff
hal_espressif zephyrproject-rtos/hal_espressif@ca3be15 zephyrproject-rtos/hal_espressif@ffbe588 (zephyr) zephyrproject-rtos/[email protected]

Note: This message is automatically posted and updated by the Manifest GitHub Action.

@bbilas bbilas force-pushed the esp32-eth-gpio-ref-clk-output branch from 3631c14 to 9276f4a Compare November 26, 2023 08:54
@zephyrbot zephyrbot added the DNM This PR should not be merged (Do Not Merge) label Nov 26, 2023
@bbilas bbilas force-pushed the esp32-eth-gpio-ref-clk-output branch from 9276f4a to 079795d Compare November 26, 2023 08:58
@bbilas bbilas requested a review from mniestroj November 26, 2023 09:01
mniestroj
mniestroj previously approved these changes Nov 26, 2023
marekmatej
marekmatej previously approved these changes Nov 27, 2023
sylvioalves
sylvioalves previously approved these changes Nov 27, 2023
@carlescufi
Copy link
Member

@bbilas please rebase

@bbilas bbilas dismissed stale reviews from sylvioalves, marekmatej, and mniestroj via 8db06ea November 28, 2023 14:51
@bbilas bbilas force-pushed the esp32-eth-gpio-ref-clk-output branch from 3091e03 to 8db06ea Compare November 28, 2023 14:51
@bbilas
Copy link
Collaborator Author

bbilas commented Nov 28, 2023

@bbilas please rebase

Done.

sylvioalves
sylvioalves previously approved these changes Nov 28, 2023
@bbilas
Copy link
Collaborator Author

bbilas commented Nov 28, 2023

@carlescufi It seems that we have not-related compliance failure, see:

 Error: See https://docs.zephyrproject.org/latest/build/kconfig/tips.html for more details.

 Found references to undefined Kconfig symbols. If any of these are false
positives, then add them to UNDEF_KCONFIG_WHITELIST in /home/runner/work/zephyr/zephyr/./scripts/ci/check_compliance.py.

If the reference is for a comment like /* CONFIG_FOO_* */ (or
/* CONFIG_FOO_*_... */), then please use exactly that form (with the '*'). The
CI check knows not to flag it.

More generally, a reference followed by $, @, {, *, or ## will never be
flagged.

CONFIG_ZTEST_NEW_API                       tests/drivers/console_switching/prj.conf:2
Error: Process completed with exit code 1.

@bbilas bbilas force-pushed the esp32-eth-gpio-ref-clk-output branch from 8db06ea to 14cc8f6 Compare November 28, 2023 14:59
@bbilas
Copy link
Collaborator Author

bbilas commented Nov 28, 2023

Rebased with main once again...

mniestroj
mniestroj previously approved these changes Nov 28, 2023
marekmatej
marekmatej previously approved these changes Nov 28, 2023
@nashif
Copy link
Member

nashif commented Nov 29, 2023

needs rebase

It contains a fix for a typo within emac_hal module.

Signed-off-by: Bartosz Bilas <[email protected]>
In case of boards where REF_CLK signal is not connected
to the GPIO0 by default add the possibility to use
the optional GPIO16/GPIO17 as a REF CLK source.

Signed-off-by: Bartosz Bilas <[email protected]>
@bbilas bbilas dismissed stale reviews from marekmatej, mniestroj, and sylvioalves via 9dad825 November 29, 2023 07:29
@bbilas bbilas force-pushed the esp32-eth-gpio-ref-clk-output branch from 14cc8f6 to 9dad825 Compare November 29, 2023 07:29
@bbilas
Copy link
Collaborator Author

bbilas commented Nov 29, 2023

needs rebase

Done.

@carlescufi carlescufi merged commit 55f2d72 into zephyrproject-rtos:main Nov 30, 2023
iwasz added a commit to iwasz/zephyr that referenced this pull request Jul 25, 2024
When GPIO17 or 16 is used as an external REF_CLK signal, the output is
enabled in eth_esp32.c This was added in PR number zephyrproject-rtos#65759 and then refined
in PR zephyrproject-rtos#74442. However this does not work for PHYs which need the REF_CLK
for MDIO communication, such as LAN8720A. In such cases phy_mii driver
tries to get the ID of such a PHY before REF_CLK is present. Therefore
in this PR I propose to move REF_CLK initialization from eth_esp32.c to
mdio_esp32.c which gets initialized before PHY and ETH.

Signed-off-by: Łukasz Iwaszkiewicz <[email protected]>
kartben pushed a commit that referenced this pull request Apr 23, 2025
When GPIO17 or 16 is used as an external REF_CLK signal, the output is
enabled in eth_esp32.c This was added in PR number #65759 and then refined
in PR #74442. However this does not work for PHYs which need the REF_CLK
for MDIO communication, such as LAN8720A. In such cases phy_mii driver
tries to get the ID of such a PHY before REF_CLK is present. Therefore
in this PR I propose to move REF_CLK initialization from eth_esp32.c to
mdio_esp32.c which gets initialized before PHY and ETH.

Signed-off-by: Łukasz Iwaszkiewicz <[email protected]>
owuatmosic pushed a commit to owuatmosic/zephyr that referenced this pull request Apr 24, 2025
When GPIO17 or 16 is used as an external REF_CLK signal, the output is
enabled in eth_esp32.c This was added in PR number zephyrproject-rtos#65759 and then refined
in PR zephyrproject-rtos#74442. However this does not work for PHYs which need the REF_CLK
for MDIO communication, such as LAN8720A. In such cases phy_mii driver
tries to get the ID of such a PHY before REF_CLK is present. Therefore
in this PR I propose to move REF_CLK initialization from eth_esp32.c to
mdio_esp32.c which gets initialized before PHY and ETH.

Signed-off-by: Łukasz Iwaszkiewicz <[email protected]>
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.

7 participants