-
Notifications
You must be signed in to change notification settings - Fork 8k
drivers: ethernet: stm32: Fix the init call order #97097
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
drivers: ethernet: stm32: Fix the init call order #97097
Conversation
drivers/ethernet/eth_stm32_hal.c
Outdated
* properly initialized. In auto-negotiation mode, it reads the speed | ||
* and duplex settings to configure the driver accordingly. | ||
*/ | ||
eth_init_api_v2(dev); |
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.
Could you test the return value?
By the way, would it make sense to move thee API_V1 also into a eth_init_api_v1()
helper function?
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.
I added a check and moved the v1 init in its own function
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.
Note there's an extra space char in your Signed-off-by tag (between your name and e-mail address) Compliance Checks CI test complains about.
60c59d4
to
558de73
Compare
drivers/ethernet/eth_stm32_hal.c
Outdated
/* HAL Init time out. This could be linked to */ | ||
/* a recoverable error. Log the issue and continue */ | ||
/* driver initialisation */ |
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.
By the way, I think it would be nice to fix the inline comment style:
/* HAL Init time out. This could be linked to
* a recoverable error. Log the issue and continue
* driver initialization.
*/
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.
(feel free to discard my comment if you prefer)
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.
I also fixed it in the v2 function
Fix the init call order for V2 API to be able to configure PTP as PTP initialization happens before iface initialization. Signed-off-by: Julien Racki <julien.racki-ext@st.com>
Move the Ethernet API v1 Initialization in eth_init_api_v1() helper function to match the V2 implementation. Signed-off-by: Julien Racki <julien.racki-ext@st.com>
|
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.
Little nit, but fine for a fix, that will only be applied to a past version and not to main.
#if defined(CONFIG_ETH_STM32_HAL_API_V1) | ||
HAL_StatusTypeDef hal_ret = HAL_OK; | ||
|
||
if (!ETH_STM32_AUTO_NEGOTIATION_ENABLE) { | ||
struct phy_link_state state; | ||
|
||
phy_get_link_state(eth_stm32_phy_dev, &state); | ||
|
||
heth->Init.DuplexMode = PHY_LINK_IS_FULL_DUPLEX(state.speed) ? ETH_MODE_FULLDUPLEX | ||
: ETH_MODE_HALFDUPLEX; | ||
heth->Init.Speed = | ||
PHY_LINK_IS_SPEED_100M(state.speed) ? ETH_SPEED_100M : ETH_SPEED_10M; | ||
} | ||
ret = eth_init_api_v1(dev); | ||
#elif defined(CONFIG_ETH_STM32_HAL_API_V2) | ||
ret = eth_init_api_v2(dev); |
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.
could share a common function name
@dkalowsk, @danieldegrasse FYI, this one is a bit specific as no backport possible. |
6da1cbc
into
zephyrproject-rtos:v4.2-branch
Fix the init call order for V2 API to be able to configure PTP as PTP initialization happens before iface initialization.
Fixes #96197
This solution is the equivalent to the one used in the main branch at this commit: ca6e25a, but it can't be directly applied in the v4.2 branch, as the Ethernet driver is not split between v1 and v2 yet in this version.