Skip to content

drivers: ethernet: stm32: HAL return value handling #97281

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.

Sign up for GitHub

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
+42 −19
Open

drivers: ethernet: stm32: HAL return value handling #97281

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9cf71bb
drivers: ethernet: stm32: test HAL return value
etienne-lms Oct 9, 2025
a664bdc
drivers: ethernet: stm32: clean HAL_ETH_{Set|Get}DMAError() value test
etienne-lms Oct 9, 2025
fa10687
drivers: ethernet: stm32: add missing empty line
etienne-lms Oct 9, 2025
07f54a5
drivers: mdio: stm32: don't mix HAL return value and errno
etienne-lms Oct 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions drivers/ethernet/eth_stm32_hal_v1.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,10 +216,14 @@ int eth_stm32_hal_init(const struct device *dev)
k_mutex_init(&dev_data->tx_mutex);
k_sem_init(&dev_data->rx_int_sem, 0, K_SEM_MAX_LIMIT);

HAL_ETH_DMATxDescListInit(heth, dma_tx_desc_tab,
&dma_tx_buffer[0][0], ETH_TXBUFNB);
HAL_ETH_DMARxDescListInit(heth, dma_rx_desc_tab,
&dma_rx_buffer[0][0], ETH_RXBUFNB);
if (HAL_ETH_DMATxDescListInit(heth, dma_tx_desc_tab,
&dma_tx_buffer[0][0], ETH_TXBUFNB) != HAL_OK) {
return -EIO;
}
if (HAL_ETH_DMARxDescListInit(heth, dma_rx_desc_tab,
&dma_rx_buffer[0][0], ETH_RXBUFNB) != HAL_OK) {
return -EIO;
}

return 0;
}
Expand All @@ -229,6 +233,7 @@ void eth_stm32_set_mac_config(const struct device *dev, struct phy_link_state *s
struct eth_stm32_hal_dev_data *dev_data = dev->data;
ETH_HandleTypeDef *heth = &dev_data->heth;
HAL_StatusTypeDef hal_ret = HAL_OK;

heth->Init.DuplexMode =
PHY_LINK_IS_FULL_DUPLEX(state->speed) ? ETH_MODE_FULLDUPLEX : ETH_MODE_HALFDUPLEX;

Expand Down
40 changes: 29 additions & 11 deletions drivers/ethernet/eth_stm32_hal_v2.c
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo in commit message for drivers: ethernet: stm32: clean HAL_ETH_{Set|Get}DMAError() value test

retrun --> return

Copy link
Contributor

@mathieuchopstm mathieuchopstm Oct 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

and I guess test against 0 ==> against HAL_OK too nvm, it returns a u32

Original file line number Diff line number Diff line change
Expand Up @@ -134,18 +134,22 @@ static inline struct eth_stm32_tx_context *allocate_tx_context(struct net_pkt *p

void eth_stm32_setup_mac_filter(ETH_HandleTypeDef *heth)
{
__ASSERT_NO_MSG(heth != NULL);
ETH_MACFilterConfigTypeDef MACFilterConf;
HAL_StatusTypeDef __maybe_unused hal_ret;

__ASSERT_NO_MSG(heth != NULL);

HAL_ETH_GetMACFilterConfig(heth, &MACFilterConf);
hal_ret = HAL_ETH_GetMACFilterConfig(heth, &MACFilterConf);
__ASSERT_NO_MSG(hal_ret == HAL_OK);

MACFilterConf.HashMulticast =
IS_ENABLED(CONFIG_ETH_STM32_MULTICAST_FILTER) ? ENABLE : DISABLE;
MACFilterConf.PassAllMulticast =
IS_ENABLED(CONFIG_ETH_STM32_MULTICAST_FILTER) ? DISABLE : ENABLE;
MACFilterConf.HachOrPerfectFilter = DISABLE;

HAL_ETH_SetMACFilterConfig(heth, &MACFilterConf);
hal_ret = HAL_ETH_SetMACFilterConfig(heth, &MACFilterConf);
__ASSERT_NO_MSG(hal_ret == HAL_OK);

k_sleep(K_MSEC(1));
}
Expand Down Expand Up @@ -183,7 +187,9 @@ int eth_stm32_tx(const struct device *dev, struct net_pkt *pkt)
net_pkt_is_tx_timestamping(pkt);
if (timestamped_frame) {
/* Enable transmit timestamp */
HAL_ETH_PTP_InsertTxTimestamp(heth);
if (HAL_ETH_PTP_InsertTxTimestamp(heth) != HAL_OK) {
return -EIO;
}
}
#endif /* CONFIG_PTP_CLOCK_STM32_HAL */

Expand Down Expand Up @@ -251,7 +257,7 @@ int eth_stm32_tx(const struct device *dev, struct net_pkt *pkt)
}

/* Check for DMA errors */
if (HAL_ETH_GetDMAError(heth)) {
if (HAL_ETH_GetDMAError(heth) != 0U) {
LOG_ERR("%s: ETH DMA error: dmaerror:%x",
__func__,
HAL_ETH_GetDMAError(heth));
Expand All @@ -260,7 +266,7 @@ int eth_stm32_tx(const struct device *dev, struct net_pkt *pkt)
}

/* Check for MAC errors */
if (HAL_ETH_GetMACError(heth)) {
if (HAL_ETH_GetMACError(heth) != 0U) {
LOG_ERR("%s: ETH MAC error: macerror:%x",
__func__,
HAL_ETH_GetMACError(heth));
Expand All @@ -276,7 +282,9 @@ int eth_stm32_tx(const struct device *dev, struct net_pkt *pkt)

if (!ctx) {
/* The HAL owns the tx context */
HAL_ETH_ReleaseTxPacket(heth);
if (HAL_ETH_ReleaseTxPacket(heth) != HAL_OK) {
return -EIO;
}
} else {
/* We need to release the tx context and its buffers */
HAL_ETH_TxFreeCallback((uint32_t *)ctx);
Expand Down Expand Up @@ -529,7 +537,12 @@ void eth_stm32_set_mac_config(const struct device *dev, struct phy_link_state *s
HAL_StatusTypeDef hal_ret = HAL_OK;
ETH_MACConfigTypeDef mac_config = {0};

HAL_ETH_GetMACConfig(heth, &mac_config);
hal_ret = HAL_ETH_GetMACConfig(heth, &mac_config);
if (hal_ret != HAL_OK) {
LOG_ERR("HAL_ETH_GetMACConfig failed: %d", hal_ret);
__ASSERT_NO_MSG(0);
return;
}

mac_config.DuplexMode =
PHY_LINK_IS_FULL_DUPLEX(state->speed) ? ETH_FULLDUPLEX_MODE : ETH_HALFDUPLEX_MODE;
Expand All @@ -541,7 +554,8 @@ void eth_stm32_set_mac_config(const struct device *dev, struct phy_link_state *s

hal_ret = HAL_ETH_SetMACConfig(heth, &mac_config);
if (hal_ret != HAL_OK) {
LOG_ERR("HAL_ETH_SetMACConfig: failed: %d", hal_ret);
LOG_ERR("HAL_ETH_SetMACConfig failed: %d", hal_ret);
__ASSERT_NO_MSG(0);
}
}

Expand Down Expand Up @@ -604,11 +618,15 @@ int eth_stm32_hal_set_config(const struct device *dev,
case ETHERNET_CONFIG_TYPE_PROMISC_MODE:
ETH_MACFilterConfigTypeDef MACFilterConf;

HAL_ETH_GetMACFilterConfig(heth, &MACFilterConf);
if (HAL_ETH_GetMACFilterConfig(heth, &MACFilterConf) != HAL_OK) {
return -EIO;
}

MACFilterConf.PromiscuousMode = config->promisc_mode ? ENABLE : DISABLE;

HAL_ETH_SetMACFilterConfig(heth, &MACFilterConf);
if (HAL_ETH_SetMACFilterConfig(heth, &MACFilterConf) != HAL_OK) {
return -EIO;
}
return 0;
#endif /* CONFIG_NET_PROMISCUOUS_MODE */
#if defined(CONFIG_ETH_STM32_MULTICAST_FILTER)
Expand Down
8 changes: 4 additions & 4 deletions drivers/mdio/mdio_stm32_hal.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ static int mdio_stm32_read(const struct device *dev, uint8_t prtad,
{
struct mdio_stm32_data *const dev_data = dev->data;
ETH_HandleTypeDef *heth = &dev_data->heth;
HAL_StatusTypeDef ret;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't it happen that prtad read fails, but regad succeeds?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same answer as for the write case, see below.

uint32_t read;
int ret;

k_sem_take(&dev_data->sem, K_FOREVER);

Expand All @@ -59,15 +59,15 @@ static int mdio_stm32_read(const struct device *dev, uint8_t prtad,

*data = read & ADIN1100_REG_VALUE_MASK;

return ret;
return 0;
}

static int mdio_stm32_write(const struct device *dev, uint8_t prtad,
uint8_t regad, uint16_t data)
{
struct mdio_stm32_data *const dev_data = dev->data;
ETH_HandleTypeDef *heth = &dev_data->heth;
int ret;
HAL_StatusTypeDef ret;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't it happen that prtad write fails, but regad succeeds?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

AFAICT, looking at the HAL functions, the PHY device and register addresses are loaded together and a timeout is returned when the full data transmission does not complete in the due time.

Another error case is when nothing can be transmitted (PHY is not ready for a new transmission) in which case none of the device and register addresses are loaded into the PHY.


k_sem_take(&dev_data->sem, K_FOREVER);

Expand All @@ -85,7 +85,7 @@ static int mdio_stm32_write(const struct device *dev, uint8_t prtad,
return -EIO;
}

return ret;
return 0;
}

#ifdef CONFIG_ETH_STM32_HAL_API_V1
Expand Down