Skip to content

Conversation

@marwaiehm-st
Copy link
Contributor

@marwaiehm-st marwaiehm-st commented Mar 4, 2025

  • Added definitions for LAN8742 PHY registers and bit masks to support auto-negotiation.
  • The function eth_init_api_v2 requires the Ethernet interface to be properly initialized. In auto-negotiation mode, it reads the speed and duplex settings to configure the driver accordingly.
  • Implemented functions to get link state and configure speed and duplex mode based on auto-negotiation results.
  • Ensured proper initialization of semaphores and MAC configuration for both auto-negotiation enabled and disabled scenarios.

@marwaiehm-st marwaiehm-st force-pushed the add_eth_autoneg_v2_rebased branch from 6f93ec2 to 1323be8 Compare March 4, 2025 14:48
@marwaiehm-st marwaiehm-st changed the title drivers: ethernet: Add API_V2 auto-negotiation support Ethernet STM32: Add API_V2 auto-negotiation support Mar 4, 2025
@marwaiehm-st marwaiehm-st force-pushed the add_eth_autoneg_v2_rebased branch 3 times, most recently from 8a9c135 to 149c715 Compare March 4, 2025 20:24
@marwaiehm-st marwaiehm-st marked this pull request as ready for review March 5, 2025 08:14
@erwango
Copy link
Member

erwango commented Mar 5, 2025

@cperera-aud Please have a look

Copy link
Member

@jukkar jukkar left a comment

Choose a reason for hiding this comment

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

There is eth_stm32_hal_get_capabilities() that returns for example the 10Mbit/sec and 100Mbit/sec support information to the caller.
I am wondering should we return in that function the current status what is being negotiated or what the driver supports in the first place. The documentation does not seem to tell it clearly, what do you think?

Never mind, it makes sense to return the driver supported capabilities here.

static void get_auto_nego_speed_duplex(ETH_HandleTypeDef *heth, ETH_MACConfigTypeDef *mac_config)
{
uint32_t phyLinkState;
uint32_t tickstart = HAL_GetTick();
Copy link
Member

Choose a reason for hiding this comment

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

Use zephyr API: k_uptime_get_32()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@marwaiehm-st
Copy link
Contributor Author

There is eth_stm32_hal_get_capabilities() that returns for example the 10Mbit/sec and 100Mbit/sec support information to the caller. I am wondering should we return in that function the current status what is being negotiated or what the driver supports in the first place. The documentation does not seem to tell it clearly, what do you think?

I think the eth_stm32_hal_get_capabilities() function should return the static capabilities of the driver, such as support for 10Mbit/sec and 100Mbit/sec. Returning the current status of the link negotiation from this function can introduce several risks:

  • Inconsistency and Confusion: The dynamic nature of link status can lead to inconsistencies and mislead callers expecting static capabilities.
  • Performance Overhead: Frequent checks for the current status can introduce unnecessary performance overhead and increased latency.

@cperera-aud
Copy link
Contributor

@cperera-aud Please have a look

@erwango , thanks for letting me know about this PR. I suppose this PR would superseed the changes in #86643 isn't it?

@marwaiehm-st
Copy link
Contributor Author

I suppose this PR would superseed the changes in #86643 isn't it?

@cperera-aud Please have a look

@erwango , thanks for letting me know about this PR. I suppose this PR would superseed the changes in #86643 isn't it?

@cperera-aud, yes, this PR does supersede the changes in #86643.

@marwaiehm-st marwaiehm-st force-pushed the add_eth_autoneg_v2_rebased branch from 149c715 to ea8807b Compare March 6, 2025 15:11
@cperera-aud
Copy link
Contributor

I suppose this PR would superseed the changes in #86643 isn't it?

@cperera-aud Please have a look

@erwango , thanks for letting me know about this PR. I suppose this PR would superseed the changes in #86643 isn't it?

@cperera-aud, yes, this PR does supersede the changes in #86643.

@marwaiehm-st , thanks for letting me know.

@erwango
Copy link
Member

erwango commented Mar 7, 2025

@cperera-aud, yes, this PR does supersede the changes in #86643.

Do you confirm this is functional on your side and answers your need ?

@cperera-aud
Copy link
Contributor

@cperera-aud, yes, this PR does supersede the changes in #86643.

Do you confirm this is functional on your side and answers your need ?

Sorry @erwango I haven't tried out the changes on my end. It would take a bit of time for us to integrate and try since we're on an old version of Zephyr. The auto negotiation is something that would certainly fit our needs.

- Added definitions for LAN8742 PHY registers and bit masks
  to support auto-negotiation.
- The function `eth_init_api_v2` requires the Ethernet interface
  to be properly initialized. In auto-negotiation mode,
  it reads the speed and duplex settings to configure
  the driver accordingly.
- Implemented functions to get link state and configure speed
  and duplex mode based on auto-negotiation results.
- Ensured proper initialization of semaphores and MAC configuration
  for both auto-negotiation enabled and disabled scenarios.

Signed-off-by: IBEN EL HADJ MESSAOUD Marwa <[email protected]>
@marwaiehm-st marwaiehm-st force-pushed the add_eth_autoneg_v2_rebased branch from ea8807b to f8e6f88 Compare March 12, 2025 10:35
@kartben kartben merged commit 18277b4 into zephyrproject-rtos:main Mar 14, 2025
22 checks passed
@ghost
Copy link

ghost commented Mar 25, 2025

Does this also fix for the H5? It did not work on my initial tests.

@marwaiehm-st
Copy link
Contributor Author

Does this also fix for the H5? It did not work on my initial tests.

Yes, what board H5 did you use?

@ghost
Copy link

ghost commented Mar 25, 2025

Does this also fix for the H5? It did not work on my initial tests.

Yes, what board H5 did you use?

stm32h573i_dk

Is a special dt-overlay and/or config needed?

@marwaiehm-st
Copy link
Contributor Author

Does this also fix for the H5? It did not work on my initial tests.

Yes, what board H5 did you use?

stm32h573i_dk

Is a special dt-overlay and/or config needed?

There's no need for any special dt-overlay or configuration. I just wanted to make sure I have the board. I'll perform the test on my side to gain a better understanding of the problem.
Did you tested the sample dhcpv_client or did you have your own test?

@ghost
Copy link

ghost commented Mar 25, 2025

Did you tested the sample dhcpv_client or did you have your own test?

Custom test. Thank you.

@marwaiehm-st
Copy link
Contributor Author

Did you tested the sample dhcpv_client or did you have your own test?

Custom test. Thank you.

Testing with #87593 may be able to solve your issue.

kevinior added a commit to kevinior/zephyr that referenced this pull request Apr 9, 2025
After API_V2 auto-negotiation support was added by zephyrproject-rtos#86621
setup_mac_filter() was called before HAL_ETH_Init(), which resulted in
received multicast packets being discarded.

This removes that call to setup_mac_filter() when API_V2 in enabled and
calls it from eth_init_api_v2() instead, after HAL_ETH_Init().

I've verified the problem and tested the fix on a Nucleo-H563ZI with a
simple application that just starts up and makes an mDNS query (which
depends on working multicast).

Signed-off-by: Kevin ORourke <[email protected]>
kevinior added a commit to kevinior/zephyr that referenced this pull request Apr 9, 2025
After API_V2 auto-negotiation support was added by zephyrproject-rtos#86621
setup_mac_filter() was called before HAL_ETH_Init(), which resulted in
received multicast packets being discarded.

This removes that call to setup_mac_filter() when API_V2 in enabled and
calls it from eth_init_api_v2() instead, after HAL_ETH_Init().

I've verified the problem and tested the fix on a Nucleo-H563ZI with a
simple application that just starts up and makes an mDNS query (which
depends on working multicast).

Signed-off-by: Kevin ORourke <[email protected]>
kevinior added a commit to kevinior/zephyr that referenced this pull request Apr 10, 2025
After API_V2 auto-negotiation support was added by zephyrproject-rtos#86621
setup_mac_filter() was called before HAL_ETH_Init(), which resulted in
received multicast packets being discarded.

This removes that call to setup_mac_filter() when API_V2 in enabled and
calls it from eth_init_api_v2() instead, after HAL_ETH_Init().

I've verified the problem and tested the fix on a Nucleo-H563ZI with a
simple application that just starts up and makes an mDNS query (which
depends on working multicast).

Signed-off-by: Kevin ORourke <[email protected]>
kevinior added a commit to kevinior/zephyr that referenced this pull request Apr 10, 2025
After API_V2 auto-negotiation support was added by zephyrproject-rtos#86621
setup_mac_filter() was called before HAL_ETH_Init(), which resulted in
received multicast packets being discarded.

This removes that call to setup_mac_filter() when API_V2 in enabled and
calls it from eth_init_api_v2() instead, after HAL_ETH_Init().

I've verified the problem and tested the fix on a Nucleo-H563ZI with a
simple application that just starts up and makes an mDNS query (which
depends on working multicast).

Signed-off-by: Kevin ORourke <[email protected]>
ghost pushed a commit to endresshauser-lp/zephyr that referenced this pull request Apr 10, 2025
After API_V2 auto-negotiation support was added by zephyrproject-rtos#86621
setup_mac_filter() was called before HAL_ETH_Init(), which resulted in
received multicast packets being discarded.

This removes that call to setup_mac_filter() when API_V2 in enabled and
calls it from eth_init_api_v2() instead, after HAL_ETH_Init().

I've verified the problem and tested the fix on a Nucleo-H563ZI with a
simple application that just starts up and makes an mDNS query (which
depends on working multicast).

Signed-off-by: Kevin ORourke <[email protected]>
ghost pushed a commit to endresshauser-lp/zephyr that referenced this pull request Apr 10, 2025
After API_V2 auto-negotiation support was added by zephyrproject-rtos#86621
setup_mac_filter() was called before HAL_ETH_Init(), which resulted in
received multicast packets being discarded.

This removes that call to setup_mac_filter() when API_V2 in enabled and
calls it from eth_init_api_v2() instead, after HAL_ETH_Init().

I've verified the problem and tested the fix on a Nucleo-H563ZI with a
simple application that just starts up and makes an mDNS query (which
depends on working multicast).

Signed-off-by: Kevin ORourke <[email protected]>
kevinior added a commit to kevinior/zephyr that referenced this pull request Apr 11, 2025
After API_V2 auto-negotiation support was added by zephyrproject-rtos#86621
setup_mac_filter() was called before HAL_ETH_Init(), which resulted in
received multicast packets being discarded.

This moves the call to setup_mac_filter() to eth_iface_init(), after
HAL_ETH_Init() for both API_V1 and API_V2.

I've verified the problem and tested the fix on a Nucleo-H563ZI with a
simple application that just starts up and makes an mDNS query (which
depends on working multicast).

Signed-off-by: Kevin ORourke <[email protected]>
kartben pushed a commit that referenced this pull request Apr 11, 2025
After API_V2 auto-negotiation support was added by #86621
setup_mac_filter() was called before HAL_ETH_Init(), which resulted in
received multicast packets being discarded.

This moves the call to setup_mac_filter() to eth_iface_init(), after
HAL_ETH_Init() for both API_V1 and API_V2.

I've verified the problem and tested the fix on a Nucleo-H563ZI with a
simple application that just starts up and makes an mDNS query (which
depends on working multicast).

Signed-off-by: Kevin ORourke <[email protected]>
ArunmaniAlagarsamy2710 pushed a commit to ArunmaniAlagarsamy2710/zephyr that referenced this pull request Apr 15, 2025
After API_V2 auto-negotiation support was added by zephyrproject-rtos#86621
setup_mac_filter() was called before HAL_ETH_Init(), which resulted in
received multicast packets being discarded.

This moves the call to setup_mac_filter() to eth_iface_init(), after
HAL_ETH_Init() for both API_V1 and API_V2.

I've verified the problem and tested the fix on a Nucleo-H563ZI with a
simple application that just starts up and makes an mDNS query (which
depends on working multicast).

Signed-off-by: Kevin ORourke <[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